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

refactor(x/group): spin out Go module #17937

Merged
merged 8 commits into from
Oct 20, 2023
Merged

refactor(x/group): spin out Go module #17937

merged 8 commits into from
Oct 20, 2023

Conversation

odeke-em
Copy link
Collaborator

@odeke-em odeke-em commented Oct 1, 2023

This change carves out cosmossdk.io/x/group as
an own Go module.

Updates #11899
Fixes #17936

Summary by CodeRabbit

  • Refactor: The group module has been moved to its own go.mod file, improving the modularity and maintainability of the codebase. The new import path is cosmossdk.io/x/group.
  • New Feature: A new job test-x-group has been added to the workflow, enhancing the testing process by running tests and analysis on the x/group directory.
  • Bug Fix: The x/consensus package now returns an error in the ToProtoConsensusParams() function, improving error handling.
  • Refactor: The x/slashing package's NewValidatorSigningInfo function now takes strings instead of sdk.AccAddress, simplifying the function's usage.
  • Documentation: A new template for a changelog file has been added to the x/group module, providing a structured way to document changes.
  • Test: The addition of the group module in the test files enhances the testing coverage of the codebase.
  • Chore: The import paths for the group module have been updated across multiple files, reflecting the module's new location.

@odeke-em odeke-em requested a review from a team as a code owner October 1, 2023 07:42
@github-prbot github-prbot requested a review from a team October 1, 2023 07:43
@github-actions github-actions bot added the C:CLI label Oct 1, 2023
@github-prbot github-prbot removed the request for review from a team October 1, 2023 07:43
@github-prbot github-prbot requested a review from atheeshp October 1, 2023 07:43
@odeke-em odeke-em force-pushed the go.mod-for-x-group branch from fd82ae2 to bff87c1 Compare October 1, 2023 07:43
go.mod Outdated Show resolved Hide resolved
@odeke-em odeke-em force-pushed the go.mod-for-x-group branch 3 times, most recently from 42c2ad6 to 4a58bff Compare October 1, 2023 07:56
x/group/go.mod Outdated Show resolved Hide resolved
Copy link
Member

@tac0turtle tac0turtle left a comment

Choose a reason for hiding this comment

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

left one question but otherwise LGTM

@odeke-em odeke-em force-pushed the go.mod-for-x-group branch from 4a58bff to 4381227 Compare October 3, 2023 08:33
@odeke-em
Copy link
Collaborator Author

odeke-em commented Oct 3, 2023

Thanks for the reviews @tac0turtle and @julienrbrt, updated!

@odeke-em odeke-em enabled auto-merge October 3, 2023 08:34
@julienrbrt julienrbrt disabled auto-merge October 3, 2023 08:38
Copy link
Member

@julienrbrt julienrbrt left a comment

Choose a reason for hiding this comment

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

Missing CHANGELOG.md + UPGRADING.md

proto/cosmos/group/module/v1/module.proto Show resolved Hide resolved
tests/go.mod Outdated Show resolved Hide resolved
x/group/go.mod Show resolved Hide resolved
x/group/sonar-project.properties Show resolved Hide resolved
Copy link
Contributor

@likhita-809 likhita-809 left a comment

Choose a reason for hiding this comment

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

same comments as julien, otherwise lgtm!

@odeke-em odeke-em force-pushed the go.mod-for-x-group branch from 4381227 to 90b7b79 Compare October 5, 2023 07:10
odeke-em added a commit to orijtech/vanity that referenced this pull request Oct 6, 2023
Adds a vanity URL entry for x/group as cosmosdk.io/x/group.
Follow-up of PR cosmos/cosmos-sdk#17937
@odeke-em odeke-em force-pushed the go.mod-for-x-group branch 2 times, most recently from 4992f26 to 06d2fa6 Compare October 6, 2023 01:35
x/group/go.mod Outdated Show resolved Hide resolved
@odeke-em odeke-em force-pushed the go.mod-for-x-group branch from 06d2fa6 to 1a2f09c Compare October 7, 2023 03:00
@tac0turtle tac0turtle requested a review from julienrbrt October 19, 2023 15:30
Copy link
Contributor

@likhita-809 likhita-809 left a comment

Choose a reason for hiding this comment

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

Tests need to be fixed. I think you need to run go mod tidy for it.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 20, 2023

Walkthrough

The changes primarily involve the migration of the group module from github.com/cosmos/cosmos-sdk/x/group to cosmossdk.io/x/group. This includes updates to import paths across multiple files, addition of new modules in the codebase, and modifications in the GitHub workflow and protobuf files. The x/auth package also sees some deprecated features removed.

Changes

Files Summary
.github/workflows/test.yml Adds a new job test-x-group to the workflow.
CHANGELOG.md Updates the changelog with notable alterations in the codebase.
api/cosmos/group/module/v1/module.pulsar.go Modifies the length of a field and updates a URL in the protobuf file.
go.work.example Adds a new module "group" in the codebase.
proto/cosmos/group/* Changes the go_package option in the protobuf files to update the import path for the generated Go code.
simapp/*.go Adds new modules and updates import paths.
tests/* Updates import paths and adds new modules.
x/group/* Updates import paths, adds new modules, and removes deprecated features.
UPGRADING.md Updates the import paths for the x/group and x/params modules.

"Hop, hop, hop, the code does swap, 🐇
From old paths to new, without a stop. 🔄
With every leap, the code does grow, 🌱
In the land of Cosmos, where the digital winds blow. 💨"


Tips

Chat with CodeRabbit Bot (@coderabbitai)

  • Mention @coderabbitai in any review comment for bot assistance.
  • Note: Review comments are made on code diffs or files, not on the PR overview.

Pause Incremental Reviews

  • Insert @coderabbitai: ignore in the PR description to halt the bot's ongoing reviews. Remove the line to resume.
  • Additionally, enter @coderabbitai pause to halt the bot's ongoing reviews as any issue or review comment. To
    resume reviews, enter @coderabbitai resume as any issue or review comment.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 0

Commits Files that changed from the base of the PR and between 4121869 and 4824628.
Files ignored due to filter (16)
  • go.mod
  • go.sum
  • simapp/go.mod
  • simapp/go.sum
  • simapp/gomod2nix.toml
  • tests/go.mod
  • tests/go.sum
  • tests/starship/tests/go.mod
  • tests/starship/tests/go.sum
  • x/group/events.pb.go
  • x/group/genesis.pb.go
  • x/group/go.mod
  • x/group/go.sum
  • x/group/query.pb.go
  • x/group/tx.pb.go
  • x/group/types.pb.go
Files selected for processing (57)
  • .github/workflows/test.yml (1 hunks)
  • CHANGELOG.md (1 hunks)
  • api/cosmos/group/module/v1/module.pulsar.go (2 hunks)
  • go.work.example (1 hunks)
  • proto/cosmos/group/module/v1/module.proto (1 hunks)
  • proto/cosmos/group/v1/events.proto (1 hunks)
  • proto/cosmos/group/v1/genesis.proto (2 hunks)
  • proto/cosmos/group/v1/query.proto (1 hunks)
  • proto/cosmos/group/v1/tx.proto (1 hunks)
  • proto/cosmos/group/v1/types.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)
  • tests/e2e/group/suite.go (2 hunks)
  • tests/integration/rapidgen/rapidgen.go (2 hunks)
  • tests/integration/tx/aminojson/aminojson_test.go (2 hunks)
  • tests/integration/tx/decode_test.go (2 hunks)
  • x/group/CHANGELOG.md (1 hunks)
  • x/group/client/cli/tx.go (1 hunks)
  • x/group/client/cli/tx_test.go (2 hunks)
  • x/group/client/cli/util.go (1 hunks)
  • x/group/internal/math/dec.go (1 hunks)
  • x/group/internal/orm/auto_uint64_test.go (1 hunks)
  • x/group/internal/orm/index.go (1 hunks)
  • x/group/internal/orm/index_test.go (1 hunks)
  • x/group/internal/orm/indexer.go (1 hunks)
  • x/group/internal/orm/indexer_test.go (1 hunks)
  • x/group/internal/orm/iterator.go (1 hunks)
  • x/group/internal/orm/iterator_property_test.go (1 hunks)
  • x/group/internal/orm/iterator_test.go (1 hunks)
  • x/group/internal/orm/key_codec.go (1 hunks)
  • x/group/internal/orm/orm_scenario_test.go (1 hunks)
  • x/group/internal/orm/primary_key_test.go (1 hunks)
  • x/group/internal/orm/sequence.go (1 hunks)
  • x/group/internal/orm/sequence_test.go (1 hunks)
  • x/group/internal/orm/table.go (1 hunks)
  • x/group/internal/orm/table_test.go (1 hunks)
  • x/group/internal/orm/types.go (1 hunks)
  • x/group/internal/orm/types_test.go (1 hunks)
  • x/group/keeper/genesis.go (1 hunks)
  • x/group/keeper/genesis_test.go (2 hunks)
  • x/group/keeper/grpc_query.go (1 hunks)
  • x/group/keeper/grpc_query_test.go (2 hunks)
  • x/group/keeper/invariants.go (1 hunks)
  • x/group/keeper/invariants_test.go (1 hunks)
  • x/group/keeper/keeper.go (1 hunks)
  • x/group/keeper/keeper_test.go (2 hunks)
  • x/group/keeper/migrations.go (1 hunks)
  • x/group/keeper/msg_server.go (1 hunks)
  • x/group/keeper/msg_server_test.go (1 hunks)
  • x/group/keeper/proposal_executor.go (1 hunks)
  • x/group/keeper/tally.go (1 hunks)
  • x/group/keeper/tally_test.go (1 hunks)
  • x/group/migrations/v2/gen_state_test.go (1 hunks)
  • x/group/migrations/v2/migrate.go (1 hunks)
  • x/group/migrations/v2/migrate_test.go (2 hunks)
Files not processed due to max files limit (15)
  • x/group/module/abci.go
  • x/group/module/abci_test.go
  • x/group/module/module.go
  • x/group/proposal_test.go
  • x/group/simulation/decoder.go
  • x/group/simulation/decoder_test.go
  • x/group/simulation/genesis.go
  • x/group/simulation/genesis_test.go
  • x/group/simulation/operations.go
  • x/group/simulation/operations_test.go
  • x/group/sonar-project.properties
  • x/group/testutil/app_config.go
  • x/group/testutil/expected_keepers.go
  • x/group/types.go
  • x/group/types_test.go
Files skipped from review due to trivial changes (34)
  • proto/cosmos/group/v1/events.proto
  • proto/cosmos/group/v1/genesis.proto
  • proto/cosmos/group/v1/query.proto
  • proto/cosmos/group/v1/tx.proto
  • proto/cosmos/group/v1/types.proto
  • tests/e2e/group/suite.go
  • tests/integration/rapidgen/rapidgen.go
  • tests/integration/tx/aminojson/aminojson_test.go
  • x/group/CHANGELOG.md
  • x/group/client/cli/tx.go
  • x/group/internal/math/dec.go
  • x/group/internal/orm/auto_uint64_test.go
  • x/group/internal/orm/index.go
  • x/group/internal/orm/index_test.go
  • x/group/internal/orm/indexer.go
  • x/group/internal/orm/indexer_test.go
  • x/group/internal/orm/key_codec.go
  • x/group/internal/orm/orm_scenario_test.go
  • x/group/internal/orm/sequence.go
  • x/group/internal/orm/table.go
  • x/group/internal/orm/table_test.go
  • x/group/internal/orm/types.go
  • x/group/internal/orm/types_test.go
  • x/group/keeper/genesis.go
  • x/group/keeper/grpc_query_test.go
  • x/group/keeper/invariants_test.go
  • x/group/keeper/keeper.go
  • x/group/keeper/msg_server.go
  • x/group/keeper/msg_server_test.go
  • x/group/keeper/proposal_executor.go
  • x/group/keeper/tally.go
  • x/group/keeper/tally_test.go
  • x/group/migrations/v2/migrate.go
  • x/group/migrations/v2/migrate_test.go
Additional comments (Suppressed): 31
CHANGELOG.md (1)
  • 173-173: The changelog entry correctly reflects the changes made in the PR. It's clear and concise, providing the necessary information about the changes to the x/group module.
go.work.example (1)
  • 23-23: The ./x/group module has been added to the go.work.example file. This change is consistent with the PR summary, which mentions that the group module has been refactored to be its own Go module. Ensure that the new path ./x/group is correct and that the module is properly set up at this location.
proto/cosmos/group/module/v1/module.proto (1)
  • 13-13: The import path for the group module has been updated to reflect its new location. Ensure that the new path "cosmossdk.io/x/group" is accessible and correctly points to the intended module.
-    go_import: "github.com/cosmos/cosmos-sdk/x/group"
+    go_import: "cosmossdk.io/x/group"
x/group/internal/orm/sequence_test.go (1)
  • 9-10: The import paths for storetypes and errors have been updated to reflect the new location of the group module. Ensure that the new paths are correct and the necessary packages are available at these locations.
x/group/client/cli/util.go (1)
  • 8-8: The import path for the group module has been updated to reflect its new location. Ensure that the new module at cosmossdk.io/x/group is correctly set up and accessible.
x/group/internal/orm/iterator_property_test.go (1)
  • 10-11: The import path for the errors package has been updated to reflect its new location. Ensure that all references to this package in the codebase have been updated accordingly.
- "github.com/cosmos/cosmos-sdk/x/group/errors"
+ "cosmossdk.io/x/group/errors"
x/group/internal/orm/iterator.go (1)
  • 9-10: The import paths for the errors package have been updated to reflect the new location of the group module. Ensure that the new paths are correct and that all references to these packages in the code have been updated accordingly.
x/group/internal/orm/primary_key_test.go (1)
  • 9-11: The import paths for errorsmod and storetypes have been updated to reflect the new location of these packages. The errors package from x/group has also been imported. Ensure that these changes do not break any dependencies and that all references to these packages in the code have been updated.
x/group/internal/orm/iterator_test.go (1)
  • 12-12: The import path for the errors package has been updated to reflect its new location. Ensure that all references to this package in the codebase have been updated accordingly.
- "github.com/cosmos/cosmos-sdk/x/group/errors"
+ "cosmossdk.io/x/group/errors"
x/group/keeper/migrations.go (1)
  • 4-4: The import path for the v2 package has been updated to reflect the new location of the group module. Ensure that the new path cosmossdk.io/x/group/migrations/v2 is correct and the package is accessible at this location.
simapp/app.go (2)
  • 24-32: The import paths for the group module have been updated to reflect its new location. Ensure that the new module at cosmossdk.io/x/group is correctly set up and accessible.

  • 95-100: The import statements for the group module have been removed from this section. This is consistent with the changes in the new hunk at lines 24-32 where the group module is now imported from its new location.

tests/integration/tx/decode_test.go (2)
  • 13-16: The import path for the group module has been updated to reflect its new location. Ensure that the new path is correct and accessible.
- groupmodule "github.com/cosmos/cosmos-sdk/x/group/module"
+ groupmodule "cosmossdk.io/x/group/module"
  • 34-39: The import statement for the group module has been removed from this section. This change is consistent with the update to the import path for the group module in lines 13-16.
x/group/client/cli/tx_test.go (2)
  • 14-22: The import paths for the group, group/client/cli, and group/module packages have been updated to reflect the new location of the group module at cosmossdk.io/x/group. Ensure that the new module location is correctly set up and accessible.

  • 29-32: No changes have been made to these import paths. Ensure that these packages are still compatible with the updated group module.

simapp/app_config.go (2)
  • 35-42: The import paths for the group module have been updated to reflect its new location at cosmossdk.io/x/group. Ensure that the new location is accessible and the module is correctly installed at this path. Also, verify that all dependencies of the group module are correctly handled in the new location.

  • 65-71: The import statement for the group module has been removed from this section. This change is consistent with the update in the previous hunk where the group module is now imported from its new location. No action is required here as long as the new import path is correctly set up and accessible.

api/cosmos/group/module/v1/module.pulsar.go (1)
  • 582-589: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [573-589]

The changes in these lines reflect the updated import paths for the group module and the errors package. The import paths have been updated from github.com/cosmos/cosmos-sdk/x/group and github.com/cosmos/cosmos-sdk/x/group/errors to cosmossdk.io/x/group and cosmossdk.io/x/group/errors respectively. This is in line with the PR summary which mentions that the group module has been refactored to be its own Go module at cosmossdk.io/x/group. Ensure that all references to the group module and the errors package in the codebase have been updated to reflect these new import paths.

simapp/app_v2.go (2)
  • 15-21: The import path for the groupkeeper has been updated to reflect the new location of the group module. Ensure that the new module at cosmossdk.io/x/group/keeper is functionally equivalent to the old one at github.com/cosmos/cosmos-sdk/x/group/keeper. Also, verify that the new module is available and accessible at the specified location.

  • 42-47: The groupkeeper import has been removed from this section. This change is consistent with the addition of the groupkeeper import in the new hunk at lines 15-21. No action is needed as long as the new groupkeeper module is functionally equivalent to the old one and is available at the new location.

x/group/keeper/grpc_query.go (1)
  • 8-17: The import paths have been updated to reflect the new location of the group module and its errors package. Ensure that the new paths are correct and that all dependencies are properly managed in the new module.
simapp/app_test.go (2)
  • 18-24: The import path for the group module has been updated to reflect its new location. Ensure that the new path is accessible and the module is correctly installed at the new location.
- group "github.com/cosmos/cosmos-sdk/x/group/module"
+ group "cosmossdk.io/x/group/module"
  • 38-43: The import statement for the group module has been removed from this section. This change is consistent with the updated import path for the group module in the previous hunk. Ensure that this does not affect any dependencies in the code.
- group "github.com/cosmos/cosmos-sdk/x/group/module"
x/group/keeper/keeper_test.go (2)
  • 13-22: The import paths for the group module and its sub-packages have been updated to reflect the new location of the module. Ensure that the new paths are correct and that the module and its sub-packages are accessible at these locations.

  • 27-31: The import paths for the group module and its sub-packages have been removed from this section, as they have been updated and moved to lines 13-22. This change is consistent with the PR summary and appears to be correct.

x/group/keeper/invariants.go (1)
  • 8-17: The import paths have been updated to reflect the new location of the group module and its sub-packages. Ensure that the new paths are correct and the packages are available at these locations.
x/group/migrations/v2/gen_state_test.go (1)
  • 9-10: The import paths for the group module and the v2 package have been updated to reflect their new locations. Ensure that the new paths are correct and accessible.
.github/workflows/test.yml (1)
  • 948-977: The new job test-x-group is added to test the x/group module. It checks for changes in the x/group directory and runs tests if there are any. It also includes SonarCloud integration for code analysis and coverage reporting. Ensure that the secrets GITHUB_TOKEN and SONAR_TOKEN are set correctly in the GitHub secrets for this repository. Also, verify that the Go version "1.21" is compatible with the rest of the codebase.
x/group/keeper/genesis_test.go (2)
  • 12-20: The import paths for the group module and its sub-packages have been updated to reflect the new location of the module. Ensure that the new paths are correct and that the module and its sub-packages are accessible at these locations.

  • 25-28: The import paths for the group module and its sub-packages have been removed as they are now imported from cosmossdk.io/x/group and its sub-packages. This change is consistent with the PR summary.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 0

Commits Files that changed from the base of the PR and between 4824628 and 071b0df.
Files selected for processing (3)
  • tests/integration/tx/aminojson/aminojson_test.go (3 hunks)
  • x/group/proposal_test.go (1 hunks)
  • x/group/simulation/decoder_test.go (1 hunks)
Files skipped from review due to trivial changes (3)
  • tests/integration/tx/aminojson/aminojson_test.go
  • x/group/proposal_test.go
  • x/group/simulation/decoder_test.go

@sonarqubecloud
Copy link

[Cosmos SDK - x/group] Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

Copy link
Member

@julienrbrt julienrbrt left a comment

Choose a reason for hiding this comment

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

lgtm!

@@ -170,6 +170,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (types) [#17885](https://github.com/cosmos/cosmos-sdk/pull/17885) `InitGenesis` & `ExportGenesis` now take `context.Context` instead of `sdk.Context`
* (x/auth) [#17985](https://github.com/cosmos/cosmos-sdk/pull/17985) Remove `StdTxConfig`
* Remove depreacted `MakeTestingEncodingParams` from `simapp/params`
* (x/group) [#17937](https://github.com/cosmos/cosmos-sdk/pull/17937) Groups module was moved to its own go.mod `cosmossdk.io/x/group`
Copy link
Member

Choose a reason for hiding this comment

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

We should add an upgrading.md as well

@tac0turtle tac0turtle added this pull request to the merge queue Oct 20, 2023
@tac0turtle tac0turtle removed this pull request from the merge queue due to a manual request Oct 20, 2023
@tac0turtle tac0turtle enabled auto-merge October 20, 2023 14:15
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 0

Commits Files that changed from the base of the PR and between 071b0df and c0997af.
Files selected for processing (1)
  • UPGRADING.md (1 hunks)
Files skipped from review due to trivial changes (1)
  • UPGRADING.md

@tac0turtle tac0turtle added this pull request to the merge queue Oct 20, 2023
Merged via the queue into main with commit 912c742 Oct 20, 2023
55 of 56 checks passed
@tac0turtle tac0turtle deleted the go.mod-for-x-group branch October 20, 2023 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

extract x/group to its own module
4 participants