refactor(x/crisis): extract as go standalone go module #18378
Conversation
WalkthroughThe changes primarily involve the migration of the "crisis" module from the "github.com/cosmos/cosmos-sdk" path to the "cosmossdk.io" path across multiple files. This includes updates to import statements, protobuf definitions, and SonarQube configurations. Additionally, a new job "test-x-crisis" has been added to the GitHub workflow. Changes
Poem
TipsChat with CodeRabbit Bot (
|
github-actions
bot
added
C:x/staking
C:x/gov
C:x/slashing
C:x/distribution
| @@ -0,0 +1,14 @@ | |||
| sonar.projectKey=cosmos-sdk-x-crisis | |||
There was a problem hiding this comment.
| @@ -1191,3 +1191,34 @@ jobs: | |||
| SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} | |||
| with: | |||
| projectBaseDir: x/mint/ | |||
|
|
|||
| test-x-crisis: | |||
There was a problem hiding this comment.
I will be making the test-x-crisis required once this PR is merged.
github-prbot
requested review from
a team,
kocubinski and
samricotta
and removed request for
a team
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to filter (9)
- go.mod
- go.sum
- simapp/go.mod
- tests/go.mod
- tests/starship/tests/go.mod
- x/crisis/go.mod
- x/crisis/go.sum
- x/crisis/types/genesis.pb.go
- x/crisis/types/tx.pb.go
Files selected for processing (30)
- .github/workflows/test.yml (1 hunks)
- api/cosmos/crisis/module/v1/module.pulsar.go (1 hunks)
- go.work.example (1 hunks)
- proto/cosmos/crisis/module/v1/module.proto (1 hunks)
- proto/cosmos/crisis/v1beta1/genesis.proto (1 hunks)
- proto/cosmos/crisis/v1beta1/tx.proto (1 hunks)
- simapp/app.go (2 hunks)
- simapp/app_config.go (2 hunks)
- simapp/app_test.go (2 hunks)
- simapp/app_v2.go (2 hunks)
- simapp/simd/cmd/commands.go (2 hunks)
- x/accounts/CHANGELOG.md (1 hunks)
- x/authz/CHANGELOG.md (1 hunks)
- x/bank/CHANGELOG.md (1 hunks)
- x/crisis/CHANGELOG.md (1 hunks)
- x/crisis/abci.go (1 hunks)
- x/crisis/keeper/genesis.go (1 hunks)
- x/crisis/keeper/genesis_test.go (1 hunks)
- x/crisis/keeper/keeper.go (1 hunks)
- x/crisis/keeper/keeper_test.go (1 hunks)
- x/crisis/keeper/msg_server.go (1 hunks)
- x/crisis/keeper/msg_server_test.go (1 hunks)
- x/crisis/module.go (2 hunks)
- x/crisis/sonar-project.properties (1 hunks)
- x/distribution/sonar-project.properties (1 hunks)
- x/gov/CHANGELOG.md (1 hunks)
- x/group/CHANGELOG.md (1 hunks)
- x/mint/CHANGELOG.md (1 hunks)
- x/slashing/CHANGELOG.md (1 hunks)
- x/staking/CHANGELOG.md (1 hunks)
Files skipped from review due to trivial changes (23)
- proto/cosmos/crisis/module/v1/module.proto
- proto/cosmos/crisis/v1beta1/genesis.proto
- proto/cosmos/crisis/v1beta1/tx.proto
- simapp/app_test.go
- simapp/app_v2.go
- simapp/simd/cmd/commands.go
- x/accounts/CHANGELOG.md
- x/authz/CHANGELOG.md
- x/bank/CHANGELOG.md
- x/crisis/CHANGELOG.md
- x/crisis/keeper/genesis.go
- x/crisis/keeper/keeper.go
- x/crisis/keeper/keeper_test.go
- x/crisis/keeper/msg_server.go
- x/crisis/keeper/msg_server_test.go
- x/crisis/module.go
- x/crisis/sonar-project.properties
- x/distribution/sonar-project.properties
- x/gov/CHANGELOG.md
- x/group/CHANGELOG.md
- x/mint/CHANGELOG.md
- x/slashing/CHANGELOG.md
- x/staking/CHANGELOG.md
Additional comments: 11
go.work.example (1)
- 24-34: The new crisis module has been added to the workspace. Ensure that all dependencies and imports are correctly updated to reflect this change.
x/crisis/keeper/genesis_test.go (2)
9-22: The import paths have been updated to reflect the new module location. Ensure that the new module is correctly set up and accessible at the new location.
9-24: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [24-28]
The
GenesisTestSuitestruct has been defined. This is a good practice as it groups related test cases together and allows for setup and teardown methods that apply to all tests in the suite.simapp/app_config.go (2)
59-66: The import paths have been updated to reflect the new location of the crisis module. Ensure that the new location is accessible and the module is correctly installed there.
69-74: The old import paths for the crisis module have been removed. Ensure that there are no remaining references to the old paths in the codebase.
x/crisis/abci.go (2)
4-11: The import paths have been updated correctly to reflect the new location of the crisis module. Ensure that the new module at "cosmossdk.io/x/crisis" is accessible and contains the expected "keeper" and "types" packages.
4-13: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [13-24]
The logic of the
EndBlockerfunction remains unchanged. It continues to check invariants at the specified interval and asserts them. No issues found here.simapp/app.go (2)
69-74: The new import paths for the crisis module look correct. Ensure that the new module at "cosmossdk.io/x/crisis" is accessible and that the codebase has been updated to reflect these changes.
107-112: No changes were made in this hunk. The import paths remain the same.
.github/workflows/test.yml (1)
- 1195-1224: The new job
test-x-crisisis well defined and follows the same pattern as other jobs in the workflow. It checks out the code, sets up Go, checks for changes in thex/crisisdirectory, runs tests if there are changes, and performs SonarCloud analysis if it's not a draft PR and theSONAR_TOKENis available. This job will help ensure that any changes to thex/crisismodule are properly tested and analyzed.api/cosmos/crisis/module/v1/module.pulsar.go (1)
- 565-592: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [0-589]
This hunk represents a protobuf definition in Go, which is a binary serialization format. It's hard to review this code because it's auto-generated and not meant to be human-readable. However, it's important to ensure that the protobuf definitions from which this code is generated are correct and up-to-date.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (12)
- simapp/app.go (2 hunks)
- simapp/app_config.go (2 hunks)
- simapp/app_test.go (2 hunks)
- simapp/app_v2.go (2 hunks)
- simapp/simd/cmd/commands.go (2 hunks)
- x/crisis/abci.go (1 hunks)
- x/crisis/keeper/genesis_test.go (1 hunks)
- x/crisis/keeper/keeper.go (1 hunks)
- x/crisis/keeper/keeper_test.go (1 hunks)
- x/crisis/keeper/msg_server.go (1 hunks)
- x/crisis/keeper/msg_server_test.go (1 hunks)
- x/crisis/module.go (2 hunks)
Files skipped from review due to trivial changes (7)
- simapp/simd/cmd/commands.go
- x/crisis/abci.go
- x/crisis/keeper/genesis_test.go
- x/crisis/keeper/keeper.go
- x/crisis/keeper/keeper_test.go
- x/crisis/keeper/msg_server_test.go
- x/crisis/module.go
Additional comments: 7
simapp/app_v2.go (1)
- 18-18: The import path for the crisis module has been updated to reflect its new location as a standalone module. Ensure that the new module is correctly set up and that all dependencies are properly managed.
simapp/app_test.go (1)
- 20-26: The import statement
"cosmossdk.io/x/crisis"has been added and the import statement"github.com/cosmos/cosmos-sdk/x/crisis"has been removed. Ensure that all references to the crisis module in this file have been updated to use the new import path.simapp/app_config.go (1)
- 35-39: The import paths for the
crisismodule have been updated to the new standalone module path. Ensure that all references to thecrisismodule in the codebase have been updated to use the new import paths.x/crisis/keeper/msg_server.go (2)
4-12: The import paths have been updated correctly. Ensure that the new paths are accessible and the packages are available at the new location.
14-14: The
Keeperstruct is correctly implementing thetypes.MsgServerinterface. This is a good practice to ensure that the struct meets the interface requirements at compile time.simapp/app.go (2)
35-37: The new import paths for the crisis module look correct. Ensure that the new module is available at the specified path and that it is compatible with the current version of the Cosmos SDK used in this project.
107-112: No changes have been made in this hunk. The import paths for the consensus and genutil modules remain the same.
| vestingtypes "github.com/cosmos/cosmos-sdk/x/auth/vesting/types" | ||
| _ "github.com/cosmos/cosmos-sdk/x/consensus" // import for side-effects | ||
| consensustypes "github.com/cosmos/cosmos-sdk/x/consensus/types" | ||
| _ "github.com/cosmos/cosmos-sdk/x/crisis" // import for side-effects | ||
| crisistypes "github.com/cosmos/cosmos-sdk/x/crisis/types" | ||
| "github.com/cosmos/cosmos-sdk/x/genutil" | ||
| genutiltypes "github.com/cosmos/cosmos-sdk/x/genutil/types" | ||
| ) |
There was a problem hiding this comment.
The import paths for the consensus module are still using the old paths. If the consensus module has also been extracted as a standalone module, its import paths should be updated as well.
- _ "github.com/cosmos/cosmos-sdk/x/consensus" // import for side-effects
- consensustypes "github.com/cosmos/cosmos-sdk/x/consensus/types"
+ _ "cosmossdk.io/x/consensus" // import for side-effects
+ consensustypes "cosmossdk.io/x/consensus/types"Commitable suggestion
[!IMPORTANT]
Carefully review the code before committing. Make sure it correctly replaces the highlighted code, has no missing lines and indentaion issues.
| vestingtypes "github.com/cosmos/cosmos-sdk/x/auth/vesting/types" | |
| _ "github.com/cosmos/cosmos-sdk/x/consensus" // import for side-effects | |
| consensustypes "github.com/cosmos/cosmos-sdk/x/consensus/types" | |
| _ "github.com/cosmos/cosmos-sdk/x/crisis" // import for side-effects | |
| crisistypes "github.com/cosmos/cosmos-sdk/x/crisis/types" | |
| "github.com/cosmos/cosmos-sdk/x/genutil" | |
| genutiltypes "github.com/cosmos/cosmos-sdk/x/genutil/types" | |
| ) | |
| vestingtypes "github.com/cosmos/cosmos-sdk/x/auth/vesting/types" | |
| _ "cosmossdk.io/x/consensus" // import for side-effects | |
| consensustypes "cosmossdk.io/x/consensus/types" | |
| "github.com/cosmos/cosmos-sdk/x/genutil" | |
| genutiltypes "github.com/cosmos/cosmos-sdk/x/genutil/types" | |
| ) |
|
[Cosmos SDK - x/crisis] Kudos, SonarCloud Quality Gate passed!
|
|
Closing this until we fix x/crisis. Then we can extract it. |
Description
I was looking at #18290 but turns out some modules still need to be extracted for this to be properly done.
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...
!to the type prefix if API or client breaking changeCHANGELOG.mdmake lintandmake testReviewers 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...
!in the type prefix if API or client breaking changeSummary by CodeRabbit