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

chore: migrate to cosmos-gogo #11414

Closed
wants to merge 32 commits into from
Closed

chore: migrate to cosmos-gogo #11414

wants to merge 32 commits into from

Conversation

robert-zaremba
Copy link
Collaborator

Description

Closes: #10925


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...

  • included the correct type prefix in the PR title
  • added ! to the type prefix if API or client breaking change
  • targeted the correct branch (see PR Targeting)
  • provided a link to the relevant issue or specification
  • followed the guidelines for building modules
  • included the necessary unit and integration tests
  • added a changelog entry to CHANGELOG.md
  • included comments for documenting Go code
  • updated the relevant documentation or specification
  • reviewed "Files changed" and left comments if necessary
  • confirmed all CI checks have passed

Reviewers 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...

  • confirmed the correct type prefix in the PR title
  • confirmed ! in the type prefix if API or client breaking change
  • confirmed all author checklist items have been addressed
  • reviewed state machine logic
  • reviewed API design and naming
  • reviewed documentation is accurate
  • reviewed tests and test coverage
  • manually tested (if applicable)

@amaury1093
Copy link
Contributor

@robert-zaremba any progress on this PR? Do you think it's worth adding it to v0.46?

@amaury1093 amaury1093 mentioned this pull request Mar 24, 2022
56 tasks
@faddat
Copy link
Contributor

faddat commented Apr 25, 2022

#11662

But I think it did not end up advancing the cause signficantly.

I will try a thing that @nguyenthevinh20000 did

The roughest issue was the interface registry's typing. Vinh solved that by forking gogo/gateway (and I figure that's necessary)

replace github.com/gogo/gateway => github.com/notional-labs/gateway v1.1.1-0.20220417180718-8e60e17a098d

is his patch, I am trying that on your branch now.

Enjoy AMS and the hackathon!

@faddat
Copy link
Contributor

faddat commented Apr 25, 2022

Hi Robert, I may have had some success here. to test that out I've got to rebuild the docker image and regenerate protos, I think (with v1.4.0)

@faddat
Copy link
Contributor

faddat commented Apr 25, 2022

I got a big improvement from comment out this line in fix_registration.go

var importsToFix = map[string]string{
	"gogo.proto": "gogoproto/gogo.proto",
	//	"cosmos.proto": "cosmos_proto/cosmos.proto",
}

it went from most tests failing to most tests passing.

Got that warm laptop test-running feeling right now...

@faddat
Copy link
Contributor

faddat commented Apr 25, 2022

#11766

But basically it all broke when I rebuilt protobufs.

I do not know how to build with the updated docker image in the devtools folder

mergify bot and others added 9 commits April 26, 2022 11:22
* feat: implement ABCI Query via gRPC (#11642)

(cherry picked from commit 3daa660)

# Conflicts:
#	CHANGELOG.md

* updates

Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com>
Co-authored-by: Aleksandr Bezobchuk <aleks.bezobchuk@gmail.com>
## Description

Closes: #11812

---

### 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...

- [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] added `!` to the type prefix if API or client breaking change
- [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#pr-targeting))
- [ ] provided a link to the relevant issue or specification
- [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/main/docs/building-modules)
- [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#testing)
- [ ] added a changelog entry to `CHANGELOG.md`
- [ ] included comments for [documenting Go code](https://blog.golang.org/godoc)
- [ ] updated the relevant documentation or specification
- [ ] reviewed "Files changed" and left comments if necessary
- [ ] confirmed all CI checks have passed

### Reviewers 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...

- [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] confirmed `!` in the type prefix if API or client breaking change
- [ ] confirmed all author checklist items have been addressed
- [ ] reviewed state machine logic
- [ ] reviewed API design and naming
- [ ] reviewed documentation is accurate
- [ ] reviewed tests and test coverage
- [ ] manually tested (if applicable)

(cherry picked from commit 8dfc205)

Co-authored-by: atheeshp <59333759+atheeshp@users.noreply.github.com>
## Description

Closes: #XXXX

---

### 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...

- [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] added `!` to the type prefix if API or client breaking change
- [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#pr-targeting))
- [ ] provided a link to the relevant issue or specification
- [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/main/docs/building-modules)
- [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#testing)
- [ ] added a changelog entry to `CHANGELOG.md`
- [ ] included comments for [documenting Go code](https://blog.golang.org/godoc)
- [ ] updated the relevant documentation or specification
- [ ] reviewed "Files changed" and left comments if necessary
- [ ] confirmed all CI checks have passed

### Reviewers 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...

- [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] confirmed `!` in the type prefix if API or client breaking change
- [ ] confirmed all author checklist items have been addressed
- [ ] reviewed state machine logic
- [ ] reviewed API design and naming
- [ ] reviewed documentation is accurate
- [ ] reviewed tests and test coverage
- [ ] manually tested (if applicable)

(cherry picked from commit 8f51644)

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>
@tac0turtle
Copy link
Member

@faddat are we okay to close this and try to work off your branch?

@robert-zaremba
Copy link
Collaborator Author

@marbar3778 we are discussing the last bits in @faddat branch , notably the additional replace.

* Update store.go

* My attempt

* fix InterfaceRegistry through changing gateway package

* Update cosmovisor/go.mod

Co-authored-by: Robert Zaremba <robert@zaremba.ch>

* Update go.mod

Co-authored-by: Robert Zaremba <robert@zaremba.ch>

* Update snapshots/chunk.go

Co-authored-by: Robert Zaremba <robert@zaremba.ch>

* Update options.go

* Update entry.go

Co-authored-by: vuong <56973102+nguyenvuong1122000@users.noreply.github.com>
Co-authored-by: nghuyenthevinh2000 <nghuyenthevinh@gmail.com>
Co-authored-by: Robert Zaremba <robert@zaremba.ch>
@faddat
Copy link
Contributor

faddat commented May 19, 2022

@marbar3778 this would be the correct place to work from.

@tac0turtle
Copy link
Member

any update here? I inclined to close it for now and we assign someone to push it over the finish line at a later date

@tac0turtle
Copy link
Member

closing this due to staleness and merge conflicts. I won't delete the branch so we can come back later to it

@tac0turtle tac0turtle closed this Jun 13, 2022
@julienrbrt julienrbrt deleted the robert/gogo branch August 18, 2022 10:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Rename github.com/gogo/protobuf fork to github.com/cosmos/gogoproto
5 participants