-
Notifications
You must be signed in to change notification settings - Fork 272
chore: use semver convention for go module #1880
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: use semver convention for go module #1880
Conversation
WalkthroughModule path and proto go_package options were moved from github.com/crypto-org-chain/cronos/v2... to github.com/crypto-org-chain/cronos... across the repository; Makefile SIMAPP and go.mod module declaration updated. Changes are import/path and proto metadata updates only. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
Makefile
(1 hunks)app/app.go
(1 hunks)go.mod
(1 hunks)x/cronos/client/cli/query.go
(1 hunks)x/cronos/client/cli/tx.go
(1 hunks)x/cronos/client/proposal_handler.go
(1 hunks)x/cronos/genesis.go
(1 hunks)x/cronos/genesis_test.go
(1 hunks)x/cronos/handler_test.go
(1 hunks)x/cronos/keeper/evm.go
(1 hunks)x/cronos/keeper/evm_hooks.go
(1 hunks)x/cronos/keeper/evm_hooks_test.go
(1 hunks)x/cronos/keeper/evm_test.go
(1 hunks)x/cronos/keeper/evmhandlers_test.go
(1 hunks)x/cronos/keeper/grpc_query.go
(1 hunks)x/cronos/keeper/ibc.go
(1 hunks)x/cronos/keeper/ibc_test.go
(1 hunks)x/cronos/keeper/keeper_test.go
(1 hunks)x/cronos/keeper/migrations.go
(1 hunks)x/cronos/keeper/msg_server.go
(1 hunks)x/cronos/keeper/params.go
(1 hunks)x/cronos/keeper/params_test.go
(1 hunks)x/cronos/keeper/permissions.go
(1 hunks)x/cronos/module.go
(1 hunks)x/cronos/proposal_handler.go
(1 hunks)x/cronos/rpc/api.go
(1 hunks)x/cronos/types/messages_test.go
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
- GitHub Check: integration_tests (ibc)
- GitHub Check: Run golangci-lint
- GitHub Check: integration_tests (gas)
- GitHub Check: integration_tests (slow)
- GitHub Check: integration_tests (ibc_update_client)
- GitHub Check: integration_tests (ica)
- GitHub Check: integration_tests (upgrade)
- GitHub Check: integration_tests (gov)
- GitHub Check: integration_tests (ibc_timeout)
- GitHub Check: integration_tests (ibc_rly_gas)
- GitHub Check: integration_tests (unmarked)
- GitHub Check: integration_tests (ibc_rly_evm)
- GitHub Check: Analyze (go)
- GitHub Check: unittest
- GitHub Check: gomod2nix
- GitHub Check: build (macos-14)
- GitHub Check: build (macos-latest)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (19)
x/cronos/genesis.go (1)
6-7
: LGTM! Import paths correctly updated.The import path migration from v2 to v1 module layout is correctly applied for both keeper and types packages.
x/cronos/proposal_handler.go (1)
4-5
: LGTM! Import paths correctly updated.The import path migration from v2 to v1 module layout is correctly applied for both keeper and types packages.
x/cronos/keeper/msg_server.go (1)
6-6
: LGTM! Import path correctly updated.The import path migration from v2 to v1 module layout is correctly applied for the types package.
x/cronos/keeper/params.go (1)
7-7
: LGTM! Import path correctly updated.The import path migration from v2 to v1 module layout is correctly applied for the types package.
x/cronos/keeper/grpc_query.go (1)
8-8
: LGTM! Import path correctly updated.The import path migration from v2 to v1 module layout is correctly applied for the types package.
x/cronos/keeper/evmhandlers_test.go (1)
8-11
: LGTM! Import paths correctly updated.All four import paths (keeper, evmhandlers, mock, types) are correctly migrated from v2 to v1 module layout.
x/cronos/genesis_test.go (1)
4-5
: LGTM! Import paths correctly updated.The import path migration from v2 to v1 module layout is correctly applied for both cronos and types packages.
x/cronos/keeper/migrations.go (1)
4-4
: LGTM! Import path correctly updated.The migration import path has been updated from
cronos/v2/x/cronos/migrations/v2
tocronos/x/cronos/migrations/v2
, aligning with the module path change while preserving thev2
alias for backward compatibility.x/cronos/types/messages_test.go (1)
10-11
: LGTM! Test imports correctly updated.Both import paths have been properly updated to the non-v2 module layout:
cmdcfg
now imports fromcronos/cmd/cronosd/config
types
now imports fromcronos/x/cronos/types
Test logic remains unchanged.
x/cronos/keeper/permissions.go (1)
4-4
: LGTM! Import path correctly updated.The types package import has been updated from
cronos/v2/x/cronos/types
tocronos/x/cronos/types
, maintaining all functionality with the new module path.go.mod (1)
1-1
: LGTM! Module path correctly downgraded to align with semantic versioning.This is the critical change that resolves the Go module error mentioned in the PR description. The module path has been correctly changed from
github.com/crypto-org-chain/cronos/v2
togithub.com/crypto-org-chain/cronos
to match the actual version (v1.x.x) being used, fixing the semver convention violation.Makefile (1)
166-166
: LGTM! SIMAPP path correctly updated.The SIMAPP variable has been updated from
github.com/crypto-org-chain/cronos/v2/app
togithub.com/crypto-org-chain/cronos/app
, ensuring simulation tests reference the correct module path after the migration.x/cronos/keeper/evm_hooks.go (1)
4-4
: LGTM! Import path correctly updated.The types package import has been updated from
cronos/v2/x/cronos/types
tocronos/x/cronos/types
. The EVM hooks logic remains unchanged and continues to reference types correctly.x/cronos/client/cli/tx.go (1)
13-13
: LGTM! Import path correctly updated.The types package import has been updated from
cronos/v2/x/cronos/types
tocronos/x/cronos/types
. All CLI command logic and message type references remain functional with the new import path.x/cronos/keeper/ibc.go (1)
10-10
: LGTM! Import path correctly updated.The types package import has been updated from
cronos/v2/x/cronos/types
tocronos/x/cronos/types
. All IBC transfer and coin conversion logic remains unchanged and continues to reference types correctly.x/cronos/client/cli/query.go (1)
7-7
: LGTM!The import path has been correctly updated to align with the module path downgrade from v2 to v1.
x/cronos/rpc/api.go (1)
10-10
: LGTM!The import path has been correctly updated to align with the module path downgrade from v2 to v1.
x/cronos/client/proposal_handler.go (1)
4-4
: LGTM!The import path has been correctly updated to align with the module path downgrade from v2 to v1.
app/app.go (1)
44-58
: LGTM!All cronos and e2ee module import paths have been consistently updated from v2 to the non-versioned path, aligning with the module path downgrade. The addition of the docs package import is also appropriate.
4da51e5
to
b210da1
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1880 +/- ##
===========================================
+ Coverage 16.87% 35.79% +18.91%
===========================================
Files 72 127 +55
Lines 6163 11812 +5649
===========================================
+ Hits 1040 4228 +3188
- Misses 5000 7161 +2161
- Partials 123 423 +300
🚀 New features to boost your workflow:
|
The module version 2 has been introduced a while ago, Is the import error seen recently? Can you describe how you import project to get this error? |
The explorer team is importing directly cronos project, it seems they were using 0.9.x before so they didnt got this issue. It seems the error happen if you try to import
|
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
x/cronos/types/cronos.pb.go
is excluded by!**/*.pb.go
x/cronos/types/genesis.pb.go
is excluded by!**/*.pb.go
x/cronos/types/query.pb.go
is excluded by!**/*.pb.go
x/cronos/types/tx.pb.go
is excluded by!**/*.pb.go
x/e2ee/types/genesis.pb.go
is excluded by!**/*.pb.go
x/e2ee/types/query.pb.go
is excluded by!**/*.pb.go
x/e2ee/types/tx.pb.go
is excluded by!**/*.pb.go
📒 Files selected for processing (74)
Makefile
(1 hunks)app/app.go
(1 hunks)app/bench_test.go
(1 hunks)app/block_address.go
(1 hunks)app/legacy.go
(1 hunks)app/prefix.go
(1 hunks)app/sim_test.go
(1 hunks)app/test_helpers.go
(1 hunks)cmd/cronosd/cmd/root.go
(1 hunks)cmd/cronosd/cmd/versiondb.go
(1 hunks)cmd/cronosd/main.go
(1 hunks)go.mod
(1 hunks)proto/cronos/cronos.proto
(1 hunks)proto/cronos/genesis.proto
(1 hunks)proto/cronos/query.proto
(1 hunks)proto/cronos/tx.proto
(1 hunks)proto/e2ee/genesis.proto
(1 hunks)proto/e2ee/query.proto
(1 hunks)proto/e2ee/tx.proto
(1 hunks)scripts/protocgen.sh
(1 hunks)testutil/simapp/simapp.go
(1 hunks)x/cronos/client/cli/query.go
(1 hunks)x/cronos/client/cli/tx.go
(1 hunks)x/cronos/client/proposal_handler.go
(1 hunks)x/cronos/events/decoders.go
(1 hunks)x/cronos/events/events.go
(1 hunks)x/cronos/genesis.go
(1 hunks)x/cronos/genesis_test.go
(1 hunks)x/cronos/handler_test.go
(1 hunks)x/cronos/keeper/evm.go
(1 hunks)x/cronos/keeper/evm_hooks.go
(1 hunks)x/cronos/keeper/evm_hooks_test.go
(1 hunks)x/cronos/keeper/evm_test.go
(1 hunks)x/cronos/keeper/evmhandlers/send_cro_to_ibc.go
(1 hunks)x/cronos/keeper/evmhandlers/send_to_account.go
(1 hunks)x/cronos/keeper/evmhandlers/send_to_ibc.go
(1 hunks)x/cronos/keeper/evmhandlers/send_to_ibc_v2.go
(1 hunks)x/cronos/keeper/evmhandlers_test.go
(1 hunks)x/cronos/keeper/grpc_query.go
(1 hunks)x/cronos/keeper/ibc.go
(1 hunks)x/cronos/keeper/ibc_test.go
(1 hunks)x/cronos/keeper/keeper.go
(1 hunks)x/cronos/keeper/keeper_test.go
(1 hunks)x/cronos/keeper/migrations.go
(1 hunks)x/cronos/keeper/msg_server.go
(1 hunks)x/cronos/keeper/msg_server_test.go
(1 hunks)x/cronos/keeper/params.go
(1 hunks)x/cronos/keeper/params_test.go
(1 hunks)x/cronos/keeper/permissions.go
(1 hunks)x/cronos/keeper/permissions_test.go
(1 hunks)x/cronos/keeper/precompiles/bank.go
(1 hunks)x/cronos/keeper/precompiles/ica.go
(1 hunks)x/cronos/keeper/precompiles/relayer.go
(1 hunks)x/cronos/middleware/conversion_middleware.go
(1 hunks)x/cronos/migrations/v2/migrate.go
(1 hunks)x/cronos/migrations/v2/migrate_test.go
(1 hunks)x/cronos/module.go
(1 hunks)x/cronos/proposal_handler.go
(1 hunks)x/cronos/rpc/api.go
(1 hunks)x/cronos/simulation/decoder.go
(1 hunks)x/cronos/simulation/decoder_test.go
(1 hunks)x/cronos/simulation/genesis.go
(1 hunks)x/cronos/simulation/genesis_test.go
(1 hunks)x/cronos/simulation/operations.go
(1 hunks)x/cronos/types/messages_test.go
(1 hunks)x/e2ee/client/cli/decrypt.go
(1 hunks)x/e2ee/client/cli/encrypt.go
(1 hunks)x/e2ee/client/cli/encrypt_to_validators.go
(1 hunks)x/e2ee/client/cli/generate.go
(1 hunks)x/e2ee/client/cli/pubkey.go
(1 hunks)x/e2ee/client/cli/query.go
(1 hunks)x/e2ee/client/cli/tx.go
(1 hunks)x/e2ee/keeper/keeper.go
(1 hunks)x/e2ee/module.go
(1 hunks)
✅ Files skipped from review due to trivial changes (17)
- x/cronos/keeper/evmhandlers/send_to_ibc_v2.go
- x/cronos/simulation/decoder_test.go
- x/cronos/keeper/evm_hooks_test.go
- x/cronos/simulation/genesis.go
- app/legacy.go
- testutil/simapp/simapp.go
- proto/cronos/cronos.proto
- x/e2ee/client/cli/query.go
- x/cronos/client/cli/query.go
- x/cronos/simulation/decoder.go
- x/cronos/keeper/evmhandlers/send_cro_to_ibc.go
- x/cronos/middleware/conversion_middleware.go
- app/prefix.go
- x/cronos/keeper/precompiles/ica.go
- x/cronos/keeper/permissions.go
- x/cronos/keeper/ibc.go
- x/cronos/events/decoders.go
🚧 Files skipped from review as they are similar to previous changes (14)
- x/cronos/keeper/evmhandlers_test.go
- x/cronos/keeper/evm_test.go
- x/cronos/keeper/evm_hooks.go
- x/cronos/genesis.go
- x/cronos/keeper/msg_server.go
- x/cronos/rpc/api.go
- x/cronos/types/messages_test.go
- x/cronos/keeper/migrations.go
- x/cronos/client/cli/tx.go
- x/cronos/genesis_test.go
- x/cronos/keeper/grpc_query.go
- x/cronos/keeper/keeper_test.go
- x/cronos/keeper/evm.go
- x/cronos/keeper/params.go
🧰 Additional context used
🪛 GitHub Actions: Protobuf
proto/e2ee/tx.proto
[error] 1-1: File option "go_package" changed from "github.com/crypto-org-chain/cronos/v2/x/e2ee/types" to "github.com/crypto-org-chain/cronos/x/e2ee/types".
proto/cronos/tx.proto
[error] 1-1: File option "go_package" changed from "github.com/crypto-org-chain/cronos/v2/x/cronos/types" to "github.com/crypto-org-chain/cronos/x/cronos/types".
proto/e2ee/genesis.proto
[error] 1-1: File option "go_package" changed from "github.com/crypto-org-chain/cronos/v2/x/e2ee/types" to "github.com/crypto-org-chain/cronos/x/e2ee/types".
proto/cronos/query.proto
[error] 1-1: File option "go_package" changed from "github.com/crypto-org-chain/cronos/v2/x/cronos/types" to "github.com/crypto-org-chain/cronos/x/cronos/types".
Makefile
[error] 330-330: proto-check-breaking failed. Make target 'proto-check-breaking' exited with code 100.
proto/e2ee/query.proto
[error] 1-1: File option "go_package" changed from "github.com/crypto-org-chain/cronos/v2/x/e2ee/types" to "github.com/crypto-org-chain/cronos/x/e2ee/types".
proto/cronos/genesis.proto
[error] 1-1: File option "go_package" changed from "github.com/crypto-org-chain/cronos/v2/x/cronos/types" to "github.com/crypto-org-chain/cronos/x/cronos/types".
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (21)
- GitHub Check: Socket Security: Pull Request Alerts
- GitHub Check: integration_tests (slow)
- GitHub Check: integration_tests (gas)
- GitHub Check: integration_tests (upgrade)
- GitHub Check: integration_tests (ica)
- GitHub Check: integration_tests (unmarked)
- GitHub Check: integration_tests (gov)
- GitHub Check: integration_tests (ibc_update_client)
- GitHub Check: integration_tests (ibc_timeout)
- GitHub Check: integration_tests (ibc)
- GitHub Check: integration_tests (ibc_rly_gas)
- GitHub Check: integration_tests (ibc_rly_evm)
- GitHub Check: build (macos-14)
- GitHub Check: unittest
- GitHub Check: gomod2nix
- GitHub Check: Analyze (go)
- GitHub Check: build (macos-latest)
- GitHub Check: build (ubuntu-latest)
- GitHub Check: Run golangci-lint
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (43)
x/cronos/keeper/permissions_test.go (1)
4-4
: LGTM! Module path correctly updated for v1 semantic versioning.The import path change from
github.com/crypto-org-chain/cronos/v2/x/cronos/keeper
togithub.com/crypto-org-chain/cronos/x/cronos/keeper
correctly aligns with Go module semantic versioning requirements. Since the project uses v1.x.x version tags (e.g., v1.5.0), the module path must not include the/v2
suffix. This resolves the reported Go module import error.x/cronos/keeper/msg_server_test.go (1)
4-5
: LGTM — all/v2
references removed and imports updated.Go imports, go.mod, and
.proto
files no longer reference/v2
.x/cronos/keeper/evmhandlers/send_to_ibc.go (1)
7-8
: LGTM — All/v2
import references removed. Verified no remaining/v2
paths in Go source or go.mod.app/bench_test.go (1)
18-18
: LGTM! Import path correctly updated.The import path has been properly migrated from the v2 module path to the non-v2 path, aligning with the PR's objective to fix Go module versioning.
x/cronos/simulation/genesis_test.go (1)
8-9
: LGTM! Import paths correctly updated.Both simulation and types package imports have been properly migrated from v2 to non-v2 module paths.
x/cronos/keeper/evmhandlers/send_to_account.go (1)
7-8
: LGTM! Import paths correctly updated.The keeper and types package imports have been properly migrated to non-v2 module paths while preserving the
cronoskeeper
alias.x/cronos/migrations/v2/migrate_test.go (1)
6-8
: LGTM! Import paths correctly updated.All three imports (exported, migrations/v2, and types) have been properly migrated to non-v2 module paths while preserving the
v2
alias for the migrations package.app/sim_test.go (1)
17-17
: LGTM! Import path correctly updated.The cronos types import has been properly migrated to the non-v2 module path while preserving the
cronosmoduletypes
alias.x/cronos/keeper/keeper.go (1)
12-13
: LGTM! Import paths correctly updated.The precompiles and types package imports have been properly migrated to non-v2 module paths while preserving the
cronosprecompiles
alias.x/cronos/keeper/ibc_test.go (1)
8-10
: LGTM! Import paths correctly updated.All three imports (keeper, keeper/mock, and types) have been properly migrated to non-v2 module paths while preserving the respective aliases. This resolves the previously flagged compilation issue.
x/cronos/keeper/precompiles/bank.go (1)
7-7
: LGTM! Import path correctly updated.The bank precompile bindings import has been properly migrated from the v2 module path to the non-v2 path.
x/cronos/simulation/operations.go (1)
7-8
: LGTM: Import paths correctly migrated.The imports now reference the non-v2 module paths, aligning with the repository-wide module path refactor.
x/cronos/migrations/v2/migrate.go (1)
4-5
: LGTM: Import paths correctly migrated.The imports now reference the non-v2 module paths, aligning with the repository-wide module path refactor.
app/test_helpers.go (1)
18-18
: LGTM: Import path correctly migrated.The import now references the non-v2 module path, aligning with the repository-wide module path refactor.
proto/e2ee/query.proto (1)
6-6
: LGTM: Protobuf go_package path correctly migrated.The
go_package
option now references the non-v2 module path, consistent with the repository-wide refactor. The pipeline failure flagging this change is expected for protobuf metadata updates.app/block_address.go (1)
6-6
: LGTM: Import path correctly migrated.The import now references the non-v2 module path, aligning with the repository-wide module path refactor.
x/cronos/handler_test.go (1)
9-11
: LGTM: Import paths correctly migrated.The imports now reference the non-v2 module paths, aligning with the repository-wide module path refactor.
scripts/protocgen.sh (1)
19-19
: LGTM: Proto file copy path correctly migrated.The source path now references the non-v2 directory structure, aligning with the repository-wide module path refactor.
x/cronos/keeper/params_test.go (1)
6-8
: LGTM: Import paths correctly migrated.All imports now reference the non-v2 module paths, resolving the incomplete migration flagged in the previous review. The changes align with the repository-wide module path refactor.
proto/cronos/query.proto (1)
11-11
: Proto go_package path update is consistent with module migration.The
go_package
option change aligns with the broader module-path refactor fromcronos/v2
tocronos
. This is a breaking change for generated Go code consumers, as flagged by the pipeline.Expected impact:
- Generated Go code will use package path
github.com/crypto-org-chain/cronos/x/cronos/types
instead ofgithub.com/crypto-org-chain/cronos/v2/x/cronos/types
.- All code importing the old generated package must update imports.
The pipeline correctly identified this as a breaking change. Ensure migration documentation covers proto-generated code package path updates.
Based on pipeline failures.
x/cronos/keeper/precompiles/relayer.go (1)
9-11
: Import paths correctly updated to align with module migration.The import paths have been updated from
cronos/v2
tocronos
(non-v2) for:
cronosevents
packageprecompile/relayer
bindings packagetypes
packageThis aligns with the broader module-path refactor. No functional changes.
proto/cronos/tx.proto (1)
11-11
: Proto go_package path update is consistent with module migration.The
go_package
option change aligns with the module-path refactor fromcronos/v2
tocronos
. This is a breaking change for generated Go code consumers, as flagged by the pipeline.The change is correct and consistent with the PR objective. Ensure migration documentation covers proto-generated code package path updates.
Based on pipeline failures.
proto/cronos/genesis.proto (1)
9-9
: Proto go_package path update is consistent with module migration.The
go_package
option change aligns with the module-path refactor fromcronos/v2
tocronos
. This is a breaking change for generated Go code consumers, as flagged by the pipeline.The change is correct and consistent with the PR objective.
Based on pipeline failures.
Makefile (2)
166-166
: SIMAPP path correctly updated to align with module migration.The
SIMAPP
variable has been updated fromgithub.com/crypto-org-chain/cronos/v2/app
togithub.com/crypto-org-chain/cronos/app
, aligning with the broader module-path refactor. This affects simulation tests that reference the app package.
330-330
: Expected proto-check-breaking failure due to intentional breaking changes.The
proto-check-breaking
target failure is expected because the PR intentionally introduces breaking changes to protogo_package
options (module path migration fromv2
to non-v2). The protobuf breaking-change detection correctly flagged these changes.Recommended actions:
- Document the breaking changes in CHANGELOG with migration instructions for proto-generated code consumers.
- Consider temporarily bypassing the proto-check-breaking check for this PR (e.g., using
--against
flag with an older branch or acknowledging the intentional break).- Ensure the next release version aligns with semantic versioning (use
v1.x.x
, notv2.x.x
).Based on pipeline failures.
proto/e2ee/tx.proto (1)
6-6
: Proto go_package path update is consistent with module migration.The
go_package
option change aligns with the module-path refactor fromcronos/v2
tocronos
for the e2ee module. This is a breaking change for generated Go code consumers, as flagged by the pipeline.The change is correct and consistent with the PR objective.
Based on pipeline failures.
proto/e2ee/genesis.proto (1)
6-6
: Proto go_package path update is consistent with module migration.The
go_package
option change aligns with the module-path refactor fromcronos/v2
tocronos
for the e2ee module. This is a breaking change for generated Go code consumers, as flagged by the pipeline.The change is correct and consistent with the PR objective.
Based on pipeline failures.
x/cronos/events/events.go (1)
6-8
: LGTM! Import path updates are consistent.The import paths have been correctly updated from the v2 module path to the non-v2 path. No functional changes detected.
app/app.go (1)
44-58
: LGTM! Comprehensive import path migration completed.All Cronos and E2EE module imports have been correctly updated from the v2 module path to the non-v2 path. The addition of the docs package import and the force-loaded RPC import are consistent with the module reorganization. No functional changes detected.
x/e2ee/client/cli/decrypt.go (1)
9-10
: LGTM! Import paths correctly updated.The e2ee keyring and types imports have been properly migrated from the v2 module path to the non-v2 path. No functional changes detected.
x/e2ee/client/cli/encrypt.go (1)
9-9
: LGTM! Import path correctly updated.The e2ee types import has been properly migrated from the v2 module path to the non-v2 path. No functional changes detected.
x/e2ee/client/cli/generate.go (1)
8-9
: LGTM! Import paths correctly updated.The e2ee keyring and types imports have been properly migrated from the v2 module path to the non-v2 path. No functional changes detected.
x/e2ee/module.go (1)
9-11
: LGTM! Module imports correctly updated.All e2ee module imports (cli, keeper, types) have been properly migrated from the v2 module path to the non-v2 path. No functional changes detected.
x/e2ee/keeper/keeper.go (1)
6-6
: LGTM! Import path correctly updated.The e2ee types import has been properly migrated from the v2 module path to the non-v2 path. No functional changes detected.
cmd/cronosd/cmd/versiondb.go (1)
9-10
: LGTM! Import paths correctly updated.The app and opendb imports have been properly migrated from the v2 module path to the non-v2 path. No functional changes detected.
x/cronos/module.go (1)
9-12
: LGTM: Import paths correctly updated to non-v2 module path.The import path changes align with the PR's objective to resolve the Go module version mismatch. All four cronos subpackage imports have been consistently updated from
github.com/crypto-org-chain/cronos/v2/x/cronos/...
togithub.com/crypto-org-chain/cronos/x/cronos/...
.x/e2ee/client/cli/encrypt_to_validators.go (1)
10-10
: LGTM: Import path correctly updated.The e2ee types import has been properly migrated to the non-v2 module path.
x/cronos/proposal_handler.go (1)
4-5
: LGTM: Import paths correctly updated.Both keeper and types package imports have been consistently migrated to the non-v2 module path.
x/e2ee/client/cli/tx.go (1)
6-6
: LGTM: Import path correctly updated.The e2ee types import has been properly migrated.
cmd/cronosd/main.go (1)
6-7
: LGTM: Import paths correctly updated.Both app and cmd package imports in the main entrypoint have been properly migrated to the non-v2 module path.
x/e2ee/client/cli/pubkey.go (1)
8-9
: LGTM: Import paths correctly updated.Both e2ee keyring and types imports have been consistently migrated to the non-v2 module path.
cmd/cronosd/cmd/root.go (1)
13-17
: LGTM: Import paths correctly updated.All four cronos-related imports (app, opendb, x/cronos, e2ee CLI) have been consistently migrated to the non-v2 module path.
x/cronos/client/proposal_handler.go (1)
4-4
: LGTM: Import path correctly updated.The cronos CLI import has been properly migrated to the non-v2 module path.
Does this import breaking changes? |
It only break projects importing cronos repository but afaik only the explorer is using it |
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.
lgtm, except the fact that clients importing this will need to know about the breaking change
Signed-off-by: Thomas <[email protected]>
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
CHANGELOG.md (1)
9-9
: Enhance CHANGELOG entry with migration guidance for consumers.The entry documents the module path change but lacks crucial context for downstream consumers who import
cronos/v2
. The entry should clearly state that this is a breaking change and include migration guidance (i.e., consumers must update their imports fromgithub.com/crypto-org-chain/cronos/v2
togithub.com/crypto-org-chain/cronos
).Consider expanding the entry to:
* [#1880](https://github.com/crypto-org-chain/cronos/pull/1880) **BREAKING**: Move module from v2 to v1 to follow semver convention. External consumers must update imports from `github.com/crypto-org-chain/cronos/v2` to `github.com/crypto-org-chain/cronos`.
Alternatively, add a dedicated "Migration Guide" section in the CHANGELOG or create a separate
MIGRATION.md
for this major breaking change.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
CHANGELOG.md
(1 hunks)go.mod
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (21)
- GitHub Check: build
- GitHub Check: gomod2nix
- GitHub Check: unittest
- GitHub Check: build (macos-14)
- GitHub Check: build (macos-latest)
- GitHub Check: build (ubuntu-latest)
- GitHub Check: Run golangci-lint
- GitHub Check: integration_tests (gov)
- GitHub Check: Analyze (go)
- GitHub Check: integration_tests (gas)
- GitHub Check: integration_tests (upgrade)
- GitHub Check: integration_tests (slow)
- GitHub Check: integration_tests (ibc_update_client)
- GitHub Check: integration_tests (ibc_rly_gas)
- GitHub Check: integration_tests (ibc_rly_evm)
- GitHub Check: integration_tests (ica)
- GitHub Check: integration_tests (ibc_timeout)
- GitHub Check: integration_tests (ibc)
- GitHub Check: integration_tests (unmarked)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (1)
go.mod (1)
1-1
: Module path change is technically sound, but verify version tagging and migration communication.The module path change from
github.com/crypto-org-chain/cronos/v2
togithub.com/crypto-org-chain/cronos
is correct and aligns with Go semantic versioning conventions (non-v2 modules use no/vN
suffix).However, this is a breaking change for all external consumers importing
cronos/v2
. A previous review flagged critical concerns:
- Version tag management: Ensure the next release uses a
v1.x.x
tag (notv2.x.x
), as the module path is now non-v2.- Migration documentation: Provide clear guidance for consumers to update imports from
github.com/crypto-org-chain/cronos/v2
togithub.com/crypto-org-chain/cronos
.- Deprecation notice: Consider adding a deprecation notice for the old
/v2
module path in the README or a dedicated migration guide.Verify that these concerns are addressed outside this file (e.g., via version tags, migration docs, or coordination with the explorer team).
ae247e5
👮🏻👮🏻👮🏻 !!!! REFERENCE THE PROBLEM YOUR ARE SOLVING IN THE PR TITLE AND DESCRIBE YOUR SOLUTION HERE !!!! DO NOT FORGET !!!! 👮🏻👮🏻👮🏻
Downgrade to v1
Prevent errors when importing the project
PR Checklist:
make
)make test
)go fmt
)golangci-lint run
)go list -json -m all | nancy sleuth
)Thank you for your code, it's appreciated! :)
Summary by CodeRabbit