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.

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

feat: get sdk response types up to standards #63

Merged
merged 11 commits into from
Jan 27, 2023

Conversation

eaddingtonwhite
Copy link
Member

  • removes signing key impl to break into its own package

Copy link
Contributor

@cprice404 cprice404 left a comment

Choose a reason for hiding this comment

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

couple nits and I think maybe get @pgautier404 to review, otherwise lgtm

// CacheGetResponse Base type for possible responses a cache GET can return. Miss || Hit
type CacheGetResponse struct {
responseType cacheGetResponseTypes
value []byte
Copy link
Contributor

Choose a reason for hiding this comment

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

so these are not accessible to users because they are lowercase, correct?

Copy link
Member Author

Choose a reason for hiding this comment

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

correct :) golang!

momento/cache_responses.go Outdated Show resolved Hide resolved
tylerburdsall
tylerburdsall previously approved these changes Jan 27, 2023
Copy link
Contributor

@tylerburdsall tylerburdsall left a comment

Choose a reason for hiding this comment

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

LGTM, but should probably get another set of eyes on this

Co-authored-by: pgautier404 <pgautier404@users.noreply.github.com>
@eaddingtonwhite eaddingtonwhite linked an issue Jan 27, 2023 that may be closed by this pull request
Copy link
Contributor

@pgautier404 pgautier404 left a comment

Choose a reason for hiding this comment

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

Looks great, thanks!

@eaddingtonwhite eaddingtonwhite merged commit 3b183c8 into main Jan 27, 2023
@eaddingtonwhite eaddingtonwhite deleted the feat/response-refactor branch January 27, 2023 21:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Get response types up to momento std's spec
4 participants