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

deps!: bump sdk #2488

Closed
wants to merge 4 commits into from
Closed

deps!: bump sdk #2488

wants to merge 4 commits into from

Conversation

shaspitz
Copy link
Contributor

@shaspitz shaspitz commented May 11, 2023

Description

Bumps sdk to https://github.com/cosmos/cosmos-sdk/releases/tag/v0.45.16-ics, containing cosmos/cosmos-sdk#16097

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

@shaspitz
Copy link
Contributor Author

Can someone sanity check me that the reason we don't need this upgrade in the ICS repo is because the sdk version dep is in a replace statement in the gaia go.mod, right?

@faddat
Copy link
Contributor

faddat commented May 13, 2023

Hey, I think this may be a better approach to the needed bump:

#2490

@mpoke
Copy link
Contributor

mpoke commented May 15, 2023

Can someone sanity check me that the reason we don't need this upgrade in the ICS repo is because the sdk version dep is in a replace statement in the gaia go.mod, right?

@smarshall-spitzbart AFAIK, the version of SDK used when building Gaia will be the one specified in the go.mod of this repo. This means that that version will be used for other dependencies as well, e.g., ICS, IBC, PFM. The reason to upgrade ICS to this new version of SDK is for testing purposes -- the entire CI of ICS works with the SDK version in the go.mod of the ICS repo. @MSalopek could you please confirm this?

@mpoke mpoke changed the title deps: bump sdk deps!: bump sdk May 15, 2023
@mpoke
Copy link
Contributor

mpoke commented May 15, 2023

@smarshall-spitzbart Note that the bump to SDK 0.45.16-ics is state machine breaking. Thus, we cannot backport to v9.0.x branch.

@mpoke
Copy link
Contributor

mpoke commented May 15, 2023

Hey, I think this may be a better approach to the needed bump:

#2490

@faddat Thanks for your feedback. Could you please motivate why you think that PR is a better approach?

Copy link
Contributor

@mpoke mpoke left a comment

Choose a reason for hiding this comment

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

Approval

@mpoke mpoke mentioned this pull request May 17, 2023
@codecov
Copy link

codecov bot commented May 17, 2023

Codecov Report

Merging #2488 (9bd5ba5) into main (9ee42c8) will increase coverage by 0.09%.
The diff coverage is 78.94%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2488      +/-   ##
==========================================
+ Coverage   84.99%   85.08%   +0.09%     
==========================================
  Files          22       23       +1     
  Lines        1599     1596       -3     
==========================================
- Hits         1359     1358       -1     
+ Misses        193      192       -1     
+ Partials       47       46       -1     
Impacted Files Coverage Δ
ante/ante.go 46.66% <ø> (ø)
app/encoding.go 100.00% <ø> (ø)
app/helpers/test_helpers.go 100.00% <ø> (ø)
app/keepers/keepers.go 98.91% <ø> (ø)
app/modules.go 100.00% <ø> (ø)
x/globalfee/ante/antetest/fee_test_setup.go 87.83% <ø> (ø)
x/globalfee/ante/fee.go 89.26% <ø> (ø)
x/globalfee/client/cli/query.go 64.51% <0.00%> (ø)
x/globalfee/keeper/migrations.go 50.00% <50.00%> (ø)
x/globalfee/module.go 81.96% <66.66%> (-1.97%) ⬇️
... and 3 more

@shaspitz shaspitz closed this May 17, 2023
@shaspitz shaspitz deleted the shawn/bump-sdk branch May 17, 2023 21:56
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