Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Introduce Store handler interface #211

Merged
merged 4 commits into from
Mar 20, 2024
Merged

Introduce Store handler interface #211

merged 4 commits into from
Mar 20, 2024

Conversation

yogeshbdeshpande
Copy link
Collaborator

@yogeshbdeshpande yogeshbdeshpande commented Mar 13, 2024 •
edited
Loading

Fixes #138

@yogeshbdeshpande yogeshbdeshpande changed the title Realm provisioning v2 Introduce Store handler interface Mar 14, 2024
@yogeshbdeshpande yogeshbdeshpande force-pushed the realm-provisioning-v2 branch 5 times, most recently from 9f3ff90 to a296cc6 Compare March 15, 2024 12:33
@yogeshbdeshpande
Add Store Interface handler
e99a0a0 
Fixes #138

Signed-off-by: Yogesh Deshpande <[email protected]>
@yogeshbdeshpande
Add more fixes
d55b061 
Signed-off-by: Yogesh Deshpande <[email protected]>
@yogeshbdeshpande yogeshbdeshpande marked this pull request as ready for review March 15, 2024 13:57
@yogeshbdeshpande yogeshbdeshpande mentioned this pull request Mar 15, 2024
Closed
thomas-fossati
thomas-fossati reviewed Mar 15, 2024
View reviewed changes
Copy link
Contributor

@thomas-fossati thomas-fossati left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks!

I have left some editorial suggestions, and a general question on the (lack of) symmetry of the store interfaces.

handler/istorehandler.go Outdated Show resolved Hide resolved
handler/istorehandler.go Show resolved Hide resolved
mk/test.mk Outdated Show resolved Hide resolved
scheme/README.md Outdated Show resolved Hide resolved
handler/error.go Show resolved Hide resolved
verification/cmd/verification-service/README.md Show resolved Hide resolved
@yogeshbdeshpande yogeshbdeshpande mentioned this pull request Mar 15, 2024
Closed
setrofim
setrofim approved these changes Mar 19, 2024
View reviewed changes
Copy link
Collaborator

@setrofim setrofim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, apart from a minor nit regarding terminology.

handler/store_rpc.go Outdated Show resolved Hide resolved
handler/store_rpc.go Outdated Show resolved Hide resolved
handler/store_rpc.go Outdated Show resolved Hide resolved
vts/cmd/vts-service/main.go Outdated Show resolved Hide resolved
@yogeshbdeshpande yogeshbdeshpande mentioned this pull request Mar 20, 2024
Closed
@yogeshbdeshpande
Incoporate further review comments
baa4b19 
Signed-off-by: Yogesh Deshpande <[email protected]>
@yogeshbdeshpande
Copy link
Collaborator Author

Looks good, apart from a minor nit regarding terminology.

Thanks @setrofim for your review. I have incorporated all your review comments in the recent commit!

@thomas-fossati thomas-fossati merged commit 899c1eb into main Mar 20, 2024
9 checks passed
@thomas-fossati thomas-fossati deleted the realm-provisioning-v2 branch March 20, 2024 17:32
thomas-fossati
thomas-fossati reviewed Mar 20, 2024
View reviewed changes
Copy link
Contributor

@thomas-fossati thomas-fossati left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thomas-fossati thomas-fossati thomas-fossati left review comments

@setrofim setrofim setrofim approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Who owns SynthKeysFrom...()?
3 participants
@yogeshbdeshpande @setrofim @thomas-fossati