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

Proposal messages are missing cosmos_proto.implements_interface option #12640

Closed
4 tasks
pyramation opened this issue Jul 19, 2022 · 2 comments
Closed
4 tasks

Proposal messages are missing cosmos_proto.implements_interface option #12640

pyramation opened this issue Jul 19, 2022 · 2 comments

Comments

@pyramation
Copy link
Contributor

pyramation commented Jul 19, 2022 •
edited
Loading

Summary

in ADR 019 it states

In .proto files:
fields which accept interfaces should be annotated with cosmos_proto.accepts_interface using the same full-qualified name passed as protoName to InterfaceRegistry.RegisterInterface
interface implementations should be annotated with cosmos_proto.implements_interface using the same full-qualified name passed as protoName to InterfaceRegistry.RegisterInterface

In the future, protoName, cosmos_proto.accepts_interface, cosmos_proto.implements_interface may be used via code generation, reflection &/or static linting.

We're doing codegen, and we're missing many of the cosmos_proto.implements_interface: cosmology-tech/telescope#118 (comment)

If we can add these into the protos, it would help the codegen flow to make client-side serialization better.

I've drafted a PR here: #12639

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@pyramation pyramation mentioned this issue Jul 19, 2022
Merged
@pyramation
Copy link
Contributor Author

pyramation commented Jul 19, 2022 •
edited
Loading

@aaronc I think you were involved in a few issues/PRs around implements_interface. Can you review the PR?

This was referenced Jul 20, 2022
Closed
Merged
mergify bot pushed a commit that referenced this issue Jul 20, 2022
@pyramation
fix: add cosmos_proto implements (#12639)
64674ba 
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
@amaury1093
Copy link
Contributor

closed by #12639

mergify bot pushed a commit that referenced this issue Jul 21, 2022
@pyramation @mergify
fix: add cosmos_proto implements (#12639)
2a3678d 
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)
amaury1093 pushed a commit that referenced this issue Jul 21, 2022
@mergify @pyramation
fix: add cosmos_proto implements (#12639) (#12664)
19d6f39 
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 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@pyramation @amaury1093