-
Notifications
You must be signed in to change notification settings - Fork 15
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(cosmos_proto): add annotation to denote version #133
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll be pushing this to https://buf.build/cosmos/cosmos-proto once this PR is merged and tag a new cosmos-proto
EDIT: After some time trying to fix #131 unsuccessfully, I am going back to this solution. |
So we can't also add this to method options and field options? That should be possible somehow |
So it seems like it gets inherited. If I add it at a lower level, it complains it is already defined. |
Are there test examples? That doesn't seem consistent with my experience. |
But does it actually work at the field level? |
Given the error message I expected it to. Adding a test. |
Maybe we try something like field_added, service_added, etc? |
little bump here :) I want to integrate it in client/v2 asap. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a lot cleaner than comments, than kyou
Method added since, etc sounds a bit ungrammatical. "Method added in" or something like that would be better |
My bad English strikes again haha. |
ServiceOptions
https://github.com/cosmos/cosmos-proto/releases/tag/v1.0.0-beta.5 I will integrate this in the SDK now (proto, api and autocli) and add docs. |
Replaces: #131
Note, if I add it in MessageOptions and FieldOptions,
buf generate
complains that it has already been defined.