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

fix: add cosmos_proto implements #12639

Merged
merged 3 commits into from
Jul 20, 2022
Merged

fix: add cosmos_proto implements #12639

merged 3 commits into from
Jul 20, 2022

Conversation

pyramation
Copy link
Contributor

@pyramation pyramation commented Jul 19, 2022

It seems that we're missing the cosmos_proto.implements_interface on many proposal messages/structs

this is a draft to discuss adding options in protos for better codegen tooling. It seems that the following messages/structs are meant to implement Content:

option (cosmos_proto.implements_interface) = "cosmos.gov.v1beta1.Content"

Would this be a breaking change? Happy to improve/amend this so it's mergeable.

related issue: #12640

@pyramation pyramation marked this pull request as ready for review July 20, 2022 00:17
@pyramation pyramation requested a review from a team as a code owner July 20, 2022 00:17
Copy link
Contributor

@alexanderbez alexanderbez left a comment

Choose a reason for hiding this comment

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

ACK. Note, these are legacy proposals and most likely will not be used going forward.

@alexanderbez alexanderbez added the A:automerge Automatically merge PR once all prerequisites pass. label Jul 20, 2022
@mergify mergify bot merged commit 64674ba into cosmos:main Jul 20, 2022
@tac0turtle
Copy link
Member

@AmauryM does this need to be back ported to 0.46?

@amaury1093
Copy link
Contributor

Maybe that's a question for @pyramation ? It's totally fine to backport it if you need it in v0.45 or v0.46

@pyramation
Copy link
Contributor Author

@AmauryM does this need to be back ported to 0.46?

we should definitely have this in 0.46 if we want codgen tooling, so I believe so!

@tac0turtle
Copy link
Member

@Mergifyio backport release/0.46.x

@mergify
Copy link
Contributor

mergify bot commented Jul 21, 2022

backport release/0.46.x

❌ No backport have been created

  • Backport to branch release/0.46.x failed: Branch not found

@tac0turtle
Copy link
Member

https://github.com/Mergifyio backport release/v0.46.x

mergify bot pushed a commit that referenced this pull request Jul 21, 2022
It seems that we're missing the `cosmos_proto.implements_interface` on many proposal messages/structs

this is a draft to discuss adding options in protos for better codegen tooling. It seems that the following messages/structs are meant to implement `Content`:

```
option (cosmos_proto.implements_interface) = "cosmos.gov.v1beta1.Content"
```

Would this be a breaking change? Happy to improve/amend this so it's mergeable.

related issue: #12640

(cherry picked from commit 64674ba)
@mergify
Copy link
Contributor

mergify bot commented Jul 21, 2022

backport release/v0.46.x

✅ Backports have been created

amaury1093 pushed a commit that referenced this pull request Jul 21, 2022
It seems that we're missing the `cosmos_proto.implements_interface` on many proposal messages/structs

this is a draft to discuss adding options in protos for better codegen tooling. It seems that the following messages/structs are meant to implement `Content`:

```
option (cosmos_proto.implements_interface) = "cosmos.gov.v1beta1.Content"
```

Would this be a breaking change? Happy to improve/amend this so it's mergeable.

related issue: #12640

(cherry picked from commit 64674ba)

Co-authored-by: Dan Lynch <pyramation@gmail.com>
@pyramation pyramation deleted the cosmos-proto-implements-interface branch July 22, 2022 18:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A:automerge Automatically merge PR once all prerequisites pass.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants