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

Update WASMD #696

Merged
merged 11 commits into from
Aug 23, 2023
Merged

Update WASMD #696

merged 11 commits into from
Aug 23, 2023

Conversation

faddat
Copy link
Contributor

@faddat faddat commented Aug 13, 2023

  • update wasmd to the latest release from confio.

Please note that this pull request no longer makes any changes to the version of go used by this repository, many thanks to @boojamya and @sontrinh16 for pointers on that.

@faddat faddat requested a review from a team as a code owner August 13, 2023 14:50
@faddat faddat requested a review from vimystic August 13, 2023 14:50
Copy link
Contributor

@boojamya boojamya left a comment

Choose a reason for hiding this comment

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

As stated here: #695 (comment), we think it is too soon to update to Go 1.21.

Could you remove the Go upgrade from this PR?

@faddat
Copy link
Contributor Author

faddat commented Aug 21, 2023

Very happy to, thanks for the note!

@faddat
Copy link
Contributor Author

faddat commented Aug 21, 2023

@boojamya hey, I think go 1.21 might've been what made this pass ci?

I am not sure of this, but check out:

9d8c2c5

@faddat
Copy link
Contributor Author

faddat commented Aug 21, 2023

I'm just going to try using 1.21 again, to check.

@faddat
Copy link
Contributor Author

faddat commented Aug 21, 2023

@boojamya -- I have a feeling that go v1.21.x is what makes this one work. I can't say precisely why, but do see fairly large changes in go.sum.

If you've other suggestions, I'm all ears.

@faddat faddat mentioned this pull request Aug 21, 2023
@faddat faddat closed this Aug 21, 2023
@faddat faddat reopened this Aug 21, 2023
@sontrinh16
Copy link

sontrinh16 commented Aug 21, 2023

@boojamya -- I have a feeling that go v1.21.x is what makes this one work. I can't say precisely why, but do see fairly large changes in go.sum.

If you've other suggestions, I'm all ears.

@faddat hey i think when you update go version in go mod file you haven't update go version in the CI script as you can see here https://github.com/faddat/ibctest/blob/271ae9e4a8a3004de48ca0716c882027c79d9c43/.github/workflows/e2e-tests.yml#L20

@faddat
Copy link
Contributor Author

faddat commented Aug 21, 2023

@sontrinh16 thank you!

I'll uptate ci, but I guess this just compounds the question of why it works with v1.21 but not v1.20.

@faddat
Copy link
Contributor Author

faddat commented Aug 21, 2023

@sontrinh16 are you referring to reverting back to 1.19?

If you think it will work please feel free to open a PR with this code and moving back to 1.19. I guess it is possible that we hit a compatibility issue.

@faddat
Copy link
Contributor Author

faddat commented Aug 22, 2023

Going to try and take all of this back to 1.19.

@sontrinh16
Copy link

@sontrinh16 thank you!

I'll uptate ci, but I guess this just compounds the question of why it works with v1.21 but not v1.20.

i think because it is using go 1.21 but the go mod file is using go 1.20 so it fail

@faddat
Copy link
Contributor Author

faddat commented Aug 22, 2023

@sontrinh16 seems to me that it really is the go version.

....which doesn't make a lot of sense.

@faddat
Copy link
Contributor Author

faddat commented Aug 22, 2023

Hey, wait -- yeah, we've flaky tests somewhere.

Evidence: this PR. Check out a few commits up, everything worked. Nothing else really changed, but sometimes we pass and sometimes we fail.

Copy link
Contributor

@boojamya boojamya left a comment

Choose a reason for hiding this comment

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

Nice. Thanks guys!

@boojamya boojamya merged commit 367bef9 into strangelove-ventures:main Aug 23, 2023
6 checks passed
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.

3 participants