-
Notifications
You must be signed in to change notification settings - Fork 607
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
Bump IAVL version from v0.19.4 -> v0.19.7 #6492
Conversation
Important Notice This PR modifies an in-repo Go module. It is one of:
The dependent Go modules, especially the root one, will have to be Please follow the instructions below:
Please let us know if you need any help. |
devbot help |
Hi! I'm DevBot, a bot that helps with common tasks in the development process. Commands:
|
devbot add changelog misc bump IAVL version to v0.19.7 |
nicejob devbot |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! One thing though that concerns me
x/ibc-hooks/go.sum
Outdated
<<<<<<< HEAD | ||
github.com/bytedance/sonic v1.10.0 h1:qtNZduETEIWJVIyDl01BeNxur2rW9OwTQ/yBqFRkKEk= | ||
github.com/bytedance/sonic v1.10.0/go.mod h1:iZcSUejdk5aukTND/Eu/ivjQuEL0Cu9/rf50Hi0u/g4= | ||
======= | ||
github.com/bytedance/sonic v1.10.1 h1:7a1wuFXL1cMy7a3f7/VFcEtriuXQnUBhtoVfOZiaysc= | ||
>>>>>>> main |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this needs to be resolved, it is weird that CI is green
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will address this. Would like to backport and test this PR this week
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I understand it, Matt pulled code and got conflict but didn't resolve it, then pushed it and Github detected those lines as code. Is this a problem of syntax error check from Github?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not think that it is a github problem, I guess the issue is that current linters used in CI are not detecting conflicts in go.sum files (I manually checked, for reference, that they do detect issues in, for example, go mod files).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So if I add a step to check for conflict markers <<<<<<<, =======, >>>>>>>
, will this case can be solved?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure at the moment what would be the best way to handle this problem. For now, I think it would be worth checking if there exists a linter that can catch these errors
If you are interested in what linters are currently enabled, I would suggest checking this file in the repo: https://github.com/osmosis-labs/osmosis/blob/main/.golangci.yml
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just one more thought: if running go mod tidy
solves this issue (i.e. it overwrites conflicts), we could add some workflow that would automatically run tidy
for every module in the repository (as a matter of fact, we already have an optimized command for doing that: #6535).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I a command to run tidy in submodules after checkout step in build.yml like
steps:
- name: Check out repository code
uses: actions/checkout@v4
- name: Run go mod tidy in all submodules
working-directory: ./
run: make tidy-workspace
will it work like when we manually run make tidy-workspace
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not think build is the best place to do this, since tidy-workspace
might change something in the repository, which is not the purpose of the build workflow.
Ideally, we should solve the bug by adding a linter that can catch it, a global go mod tidy
is a way we should go with if a linter option does not work
Currently running a node on v19 release line |
The nodee synched over epoch - merging |
* Bump sdk and IAVL version * update changelog * conflict --------- Co-authored-by: devbot-wizard <[email protected]> Co-authored-by: roman <[email protected]> (cherry picked from commit 57d5923) # Conflicts: # CHANGELOG.md # go.mod # go.sum # osmomath/go.mod # osmomath/go.sum # osmoutils/go.mod # osmoutils/go.sum # tests/cl-genesis-positions/go.mod # tests/cl-genesis-positions/go.sum # tests/cl-go-client/go.mod # tests/cl-go-client/go.sum # x/epochs/go.mod # x/epochs/go.sum # x/ibc-hooks/go.mod # x/ibc-hooks/go.sum
Co-authored-by: roman <[email protected]>
* Bump sdk and IAVL version * update changelog * conflict --------- Co-authored-by: devbot-wizard <[email protected]> Co-authored-by: roman <[email protected]>
Closes: #XXX
What is the purpose of the change
Bumps IAVL version from v0.19.4 -> v0.19.7.
Also updates sdk commit hash accordingly to the commit that uses the bumped version of IAVL
Documentation and Release Note
Unreleased
section ofCHANGELOG.md
?Where is the change documented?
x/{module}/README.md
)