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

refactor: migrate to cosmos/gogoproto #9356

Merged
merged 5 commits into from
Sep 6, 2022
Merged

refactor: migrate to cosmos/gogoproto #9356

merged 5 commits into from
Sep 6, 2022

Conversation

julienrbrt
Copy link
Contributor

@julienrbrt julienrbrt commented Sep 2, 2022

The SDK is moving away from gogo/protobuf to use our fork cosmos/gogoproto. This fork is based on Regen's fork, which we have been using for sometimes (https://github.com/cosmos/cosmos-sdk/blob/main/go.mod#L301).

Currently, we are blocked because we are importing TM types (cosmos/cosmos-sdk#13070 (comment)) and I think it would be a win for both to switch to cosmos/gogoproto as gogo/protobuf is currently not maintained anymore.


PR checklist

  • Tests written/updated, or no tests needed
  • CHANGELOG_PENDING.md updated, or no changelog entry needed
  • Updated relevant documentation (docs/) and code comments, or no
    documentation updates needed

@julienrbrt julienrbrt requested a review from a team September 2, 2022 14:29
@julienrbrt julienrbrt changed the title refacactor: migrate to cosmos/gogoproto refactor: migrate to cosmos/gogoproto Sep 2, 2022
Copy link
Contributor

@thanethomson thanethomson left a comment

Choose a reason for hiding this comment

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

Who's going to be committing to maintaining this fork over time?

Also, what differences are there between the fork and https://github.com/gogo/protobuf?

Makefile Outdated Show resolved Hide resolved
@tychoish
Copy link
Contributor

tychoish commented Sep 2, 2022

Who's going to be committing to maintaining this fork over time?

Also, what differences are there between the fork and https://github.com/gogo/protobuf?

I believe that the SDK team will be maintaining it. It is the case that gogo/protobuf is not presently maintained, so it's probably best that we move to something. The tendermint core team has long been of the opinion that Tendermint should move to using the upstream/default protobuf generation rather than any fork, but all other things being equal, the cosmos protobuf generator is probably better than gogo proto, though I'd still prefer to use stock prototobuf.

@thanethomson
Copy link
Contributor

Cool, thanks for this @julienrbrt 👍 I see some minor conflicts have come up here, but otherwise this should be good to merge.

@julienrbrt
Copy link
Contributor Author

julienrbrt commented Sep 3, 2022

Who's going to be committing to maintaining this fork over time?
Also, what differences are there between the fork and https://github.com/gogo/protobuf?

I believe that the SDK team will be maintaining it. It is the case that gogo/protobuf is not presently maintained, so it's probably best that we move to something. The tendermint core team has long been of the opinion that Tendermint should move to using the upstream/default protobuf generation rather than any fork, but all other things being equal, the cosmos protobuf generator is probably better than gogo proto, though I'd still prefer to use stock prototobuf.

Right, the fork adds as well castrepeated support (gogo/protobuf#658).

@tac0turtle
Copy link
Contributor

if this is merged in a short period could we include it in 0.37?

Copy link
Contributor

@thanethomson thanethomson left a comment

Choose a reason for hiding this comment

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

Seems reasonable to me. Thanks @julienrbrt!

@thanethomson thanethomson added the S:backport-to-v0.37.x Tell mergify to backport the PR to v0.37.x label Sep 6, 2022
@thanethomson thanethomson merged commit 101bf50 into tendermint:main Sep 6, 2022
mergify bot pushed a commit that referenced this pull request Sep 6, 2022
* refactor: migrate to `cosmos/gogoproto`

* add changelog

* Update Makefile

Co-authored-by: Thane Thomson <connect@thanethomson.com>

* update link

Co-authored-by: Thane Thomson <connect@thanethomson.com>
(cherry picked from commit 101bf50)

# Conflicts:
#	go.mod
@julienrbrt julienrbrt deleted the gogoproto branch September 6, 2022 12:05
@tac0turtle
Copy link
Contributor

thank you!!

samricotta pushed a commit that referenced this pull request Sep 7, 2022
* refactor: migrate to `cosmos/gogoproto`

* add changelog

* Update Makefile

Co-authored-by: Thane Thomson <connect@thanethomson.com>

* update link

Co-authored-by: Thane Thomson <connect@thanethomson.com>
thanethomson added a commit that referenced this pull request Sep 7, 2022
* refactor: migrate to `cosmos/gogoproto` (#9356)

* refactor: migrate to `cosmos/gogoproto`

* add changelog

* Update Makefile

Co-authored-by: Thane Thomson <connect@thanethomson.com>

* update link

Co-authored-by: Thane Thomson <connect@thanethomson.com>

* `go mod tidy`

Co-authored-by: Thane Thomson <connect@thanethomson.com>
james-chf added a commit to heliaxdev/tendermint that referenced this pull request Nov 25, 2022
…x-rc1

* release/v0.37.0-rc1:
  QA Process report for v0.37.x (and baseline for v0.34.x) (tendermint#9499) (tendermint#9577)
  Fix TX payload for DO testnets (tendermint#9540) (tendermint#9542)
  blocksync: retry requests after timeout (backport tendermint#9518) (tendermint#9533)
  Extend the load report tool to include transactions' hashes (tendermint#9509) (tendermint#9513)
  build(deps): Bump styfle/cancel-workflow-action from 0.10.0 to 0.10.1 (tendermint#9502)
  build(deps): Bump actions/stale from 5 to 6 (tendermint#9494)
  loadtime: add block time to the data point (tendermint#9484) (tendermint#9489)
  config: Add missing storage section when generating config (tendermint#9483) (tendermint#9487)
  Sync Vote.Verify() in spec with implementation (tendermint#9466) (tendermint#9476)
  fix spec (tendermint#9467) (tendermint#9469)
  metrics: fix panic because of absent prometheus label (tendermint#9455) (tendermint#9474)
  Ensure Dockerfile stages use consistent Go version (backport tendermint#9462) (tendermint#9472)
  build(deps): Bump slackapi/slack-github-action from 1.21.0 to 1.22.0 (tendermint#9432)
  build(deps): Bump bufbuild/buf-setup-action from 1.7.0 to 1.8.0 (tendermint#9453)
  state: restore previous error message (tendermint#9435) (tendermint#9440)
  build(deps): Bump gonum.org/v1/gonum from 0.11.0 to 0.12.0 (tendermint#9411)
  docs: Update ADRs for v0.37 (tendermint#9399) (tendermint#9418)
  build(deps): Bump github.com/spf13/viper from 1.12.0 to 1.13.0 (tendermint#9410)
  build(deps): Bump github.com/lib/pq from 1.10.6 to 1.10.7 (tendermint#9409)
  feat: support HTTPS inside websocket (tendermint#9416) (tendermint#9422)
  Removed unused param (tendermint#9394)
  test: generate uuid on startup for load tool (tendermint#9383) (tendermint#9392)
  add redirect links (tendermint#9385) (tendermint#9389)
  refactor: migrate to cosmos/gogoproto (backport tendermint#9356) (tendermint#9381)
  cmd: print all versions of tendermint and its sub protocols  (tendermint#9329) (tendermint#9386)
  Add missing changes changelog files (backport tendermint#9376) (tendermint#9382)
  add separated runs by UUID (backport tendermint#9367) (tendermint#9379)
  spec: abci++ cleanup for v0.37 (backport tendermint#9288) (tendermint#9374)
  ci: Remove "(WARNING: BETA SOFTWARE)" tagline from all upcoming releases (tendermint#9371) (tendermint#9372)
  Update rpc client header (tendermint#9276) (tendermint#9349)
  ci: Pre-release workflows (backport tendermint#9366) (tendermint#9368)
  test: add the loadtime report tool (tendermint#9351) (tendermint#9364)
  Update Tendermint version to v0.37.0 (tendermint#9354)
  test: add the loadtime tool (tendermint#9342) (tendermint#9357)

# Conflicts:
#	version/version.go
@thanethomson thanethomson mentioned this pull request May 6, 2023
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S:backport-to-v0.37.x Tell mergify to backport the PR to v0.37.x
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants