fix: build .proto files like any cosmos project #2462
Conversation
faddat
requested review from
alexanderbez,
yaruwangway,
crodriguezvega,
jackzampolin,
mmulji-ic,
glnro,
jtremback,
mpoke,
sainoe,
MSalopek and
shaspitz
as code owners
|
It uses buf registry, where that can apply. It doesn't use the buf registry to replace the sdk for example I think that the way to get this in, per @jtremback comments on the image builder, is to make a PR into gaia with an image builder that self-publishes to ghcr.io/cosmos/gaia -- wdyt? |
There was a problem hiding this comment.
Thanks for your contribution!
This PR contains changes other than the changes outlined in the description.
Could please revert any changes to version checking, linter installing and linting process in the Makefile?
Other than that, no concrete issues with accepting the PR.
Very soon the tooling written here will be superceeded by what's needed for cosmos-sdk > 0.45.x. This could actually make the transition easier.
|
sure thing, will address this later today @MSalopek thanks for the review! |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #2462 +/- ##
=======================================
Coverage 85.16% 85.16%
=======================================
Files 23 23
Lines 1604 1604
=======================================
Hits 1366 1366
Misses 192 192
Partials 46 46 |
| @@ -6,7 +6,7 @@ package types | |||
| import ( | |||
| context "context" | |||
| fmt "fmt" | |||
| _ "github.com/gogo/protobuf/gogoproto" | |||
| _ "github.com/cosmos/gogoproto/gogoproto" | |||
There was a problem hiding this comment.
Chiming in here. If this is for v0.45, the gogo-proto version in buf.yaml should be pinned to bee5511075b7499da6178d9e4aaa628b. (https://buf.build/cosmos/gogo-proto/file/bee5511075b7499da6178d9e4aaa628b:gogoproto/gogo.proto#L36). Latest uses our fork (https://buf.build/cosmos/gogo-proto/file/34d970b699f84aa382f3c29773a60836:gogoproto/gogo.proto#L36) and is only needed from v0.47+.
There was a problem hiding this comment.
thank you very much! I wasn't aware that there was an incompatiblity.
|
@faddat can you address the comments above. |
|
@mmulji-ic & @jtremback -- concerning Jehan's comment -- should I move that builder into this repository? |
|
This should be simple to merge once Gaia is publishing her own proto builder image :) |
|
Following up on this: Other than that the changes seem to do the job! |
|
@faddat could you please address the comments above? |
|
Converting to draft until the comments are addressed. |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Description
Gaia's proto build setup will produce different results if run on different machines. This is why the cosmos-sdk distributes a docker container for building protos inside of. This PR adopts that docker container.
https://github.com/faddat/proto-builder
is a docker image I made that contains:
If you'd like, I can add that to this PR, and we can make a second PR that removes the reference to faddat/proto-builder
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!to the type prefix if API or client breaking changeCHANGELOG.mdReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
!in the type prefix if API or client breaking change