-
Notifications
You must be signed in to change notification settings - Fork 650
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: remove all use of x/params
#7900
base: main
Are you sure you want to change the base?
Conversation
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.
Love getting rid of this <3
It looks like there might still be some x/params left somewhere as it is still in the go.mod, is there more we should remove?
@@ -10,3 +10,6 @@ const ( | |||
// ParamsKey is the store key for the interchain accounts controller parameters | |||
ParamsKey = "params" | |||
) | |||
|
|||
// KeyControllerEnabled is the store key for ControllerEnabled Params for legacy purposes. | |||
var KeyControllerEnabled = []byte("ControllerEnabled") |
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 can't see much reason for this to be left behind now?
Looking it up on this branch, it looks like it is only used in tests, which also tells me there might be more code to remove?
m.keeper.SetParams(ctx, params) | ||
m.keeper.Logger.Info("successfully migrated ica/host submodule to self-manage params") | ||
} | ||
func (Migrator) MigrateParams(_ context.Context) error { |
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.
Nah, let's just delete the whole thing.
var ( | ||
// KeyHostEnabled is the store key for HostEnabled Params for legacy purposes. | ||
// Deprecated: this will be removed. | ||
KeyHostEnabled = []byte("HostEnabled") |
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.
These too
modules/apps/callbacks/go.mod
Outdated
@@ -58,6 +57,7 @@ require ( | |||
cosmossdk.io/x/evidence v0.0.0-00010101000000-000000000000 // indirect | |||
cosmossdk.io/x/group v0.0.0-00010101000000-000000000000 // indirect | |||
cosmossdk.io/x/nft v0.0.0-00010101000000-000000000000 // indirect | |||
cosmossdk.io/x/params v0.0.0-00010101000000-000000000000 // indirect |
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.
Should there be anything left to keep this around?
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.
nope - can remove
|
||
m.keeper.SetParams(ctx, params) | ||
m.keeper.Logger.Info("successfully migrated transfer app self-manage params") | ||
func (Migrator) MigrateParams(_ context.Context) error { |
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.
Lets toss it :D
Quality Gate passed for 'ibc-go'Issues Measures |
Description
Remove the now unsupported
x/params
module in ibc-go v10. Apps moving to Cosmos SDK v0.52.x will need to removex/params
so removing it fromibc-go
will remove help remove it entirely from an app using the stack.For now, I just moved the existing
MigrateParams
functions to be a no-op, but I can remove them if desired.closes: #XXXX
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/
).godoc
comments.Files changed
in the GitHub PR explorer.SonarCloud Report
in the comment section below once CI passes.