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

close 815: upgrade to go 1.18 #866

Merged
merged 9 commits into from
Aug 24, 2022

Conversation

faddat
Copy link
Contributor

@faddat faddat commented May 18, 2022

This PR upgrades go to version 1.18

closes:

@faddat faddat requested a review from alpe as a code owner May 18, 2022 20:34
Copy link
Contributor

@alpe alpe left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. You are bleeding edge as always and I appreciate your energy and effort!

When we discussed this internally, the plan was to be a bit more conservative and follow the sdk roadmap on this.
The sdk v0.46.x and v0.45.x are still on Go 1.17.

IMHO this upgrade needs more time to settle but please comment on #815 if you have any requirements or see other chains upgrading to Go 1.18. This will help to get more confidence.

Other than this, your PR looks good. Just update the circle-ci image and Readme.

@jhernandezb
Copy link
Contributor

We have been running stargaze with go 1.18 without issues so far for a month probalby, we have not introduced any of the new features (generics) to the codebase tho.

@faddat
Copy link
Contributor Author

faddat commented May 23, 2022

Given (whatever this is)

https://app.circleci.com/pipelines/github/CosmWasm/wasmd/2111/workflows/7a47baf1-f484-4803-891d-eac1de26f0a5/jobs/7124

could we consider moving to plain github actions?

@alpe
Copy link
Contributor

alpe commented May 24, 2022

The new circleCI image is unfortunately not a drop in replacement. I have opened #872 and will 👀 when I get a chance

@faddat
Copy link
Contributor Author

faddat commented May 26, 2022

Kinda my point about migrating to actions.

Also, if we use actions, the whole repo becomes more easily forkable.

@faddat
Copy link
Contributor Author

faddat commented May 26, 2022

got it :)

@ethanfrey
Copy link
Member

@alpe what is the status here? I was happy not to merge before v0.27.0 (there was plenty in there already), but what about merging this now, to include in v0.28.0.

Cosmos-sdk v0.46.0-rc1 us on Go 1.18 and Stargaze's experience is also positive (as they use CosmWasm and the whole stack)

@ethanfrey ethanfrey added this to the v0.28.0 milestone Jun 17, 2022
Copy link
Contributor

@alpe alpe left a comment

Choose a reason for hiding this comment

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

Thank you very much for the PR! 🌻
Everything looks great except that make proto-gen is broken in this PR. If you can revert that change in protocgen.sh, I would merge this asap, now.

@@ -1,6 +1,9 @@
# Changelog

## [Unreleased](https://github.com/CosmWasm/wasmd/tree/HEAD)
- Upgrade go to v1.18 [\#866]https://github.com/CosmWasm/wasmd/pull/866/) ([faddat](https://github.com/faddat))
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@@ -13,7 +13,7 @@ This code was forked from the `cosmos/gaia` repository as a basis and then we ad
many gaia-specific files. However, the `wasmd` binary should function just like `gaiad` except for the
addition of the `x/wasm` module.

**Note**: Requires [Go 1.17+](https://golang.org/dl/)
**Note**: Requires [Go 1.18+](https://golang.org/dl/)
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 very good to have the doc up to date!

@@ -8,7 +8,7 @@ protoc_gen_gocosmos() {
return 1
fi

go get github.com/regen-network/cosmos-proto/protoc-gen-gocosmos@latest 2>/dev/null
go install github.com/regen-network/cosmos-proto/protoc-gen-gocosmos@latest 2>/dev/null
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not working with the docker image (tendermintdev/sdk-proto-gen) that we use. Can you revert this change/ test with make proto-gen ?

Copy link
Contributor

@alpe alpe left a comment

Choose a reason for hiding this comment

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

I can not expect you to work on this old PR to fix one line. It is also blocking the sdk upgrade, so I will merge it as is and provide a patch myself. Thanks again for your work!

@alpe alpe merged commit 61bda97 into CosmWasm:main Aug 24, 2022
alpe added a commit that referenced this pull request Aug 24, 2022
This was referenced Aug 24, 2022
alpe added a commit that referenced this pull request Aug 24, 2022
* Revert protocgen changes from #866

* Go mod tidy

* Bump proto-gen image one version
@faddat faddat mentioned this pull request Oct 2, 2022
conorpp pushed a commit to wormhole-foundation/wasmd that referenced this pull request Feb 1, 2023
* close 815: upgrade to go 1.18

* Update CHANGELOG.md

* updater ci and docker to go 1.18

* update image per: https://discuss.circleci.com/t/legacy-convenience-image-deprecation/41034

* Update config.yml

* go get -> go install, except dockerfile for protos
conorpp pushed a commit to wormhole-foundation/wasmd that referenced this pull request Feb 1, 2023
* Revert protocgen changes from CosmWasm#866

* Go mod tidy

* Bump proto-gen image one version
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants