chore: use semver convention for go module #1880
Conversation
thomas-nguy
requested review from
JayT106 and
randy-cro
and removed request for
a team
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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/v2tocronos/x/cronos/migrations/v2, aligning with the module path change while preserving thev2alias 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:
cmdcfgnow imports fromcronos/cmd/cronosd/configtypesnow imports fromcronos/x/cronos/typesTest 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/typestocronos/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/v2togithub.com/crypto-org-chain/cronosto 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/apptogithub.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/typestocronos/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/typestocronos/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/typestocronos/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.
thomas-nguy
force-pushed
the
thomas/downgrade-mod-version
branch
from
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:
|
thomas-nguy
changed the title
|
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.
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.gois excluded by!**/*.pb.gox/cronos/types/genesis.pb.gois excluded by!**/*.pb.gox/cronos/types/query.pb.gois excluded by!**/*.pb.gox/cronos/types/tx.pb.gois excluded by!**/*.pb.gox/e2ee/types/genesis.pb.gois excluded by!**/*.pb.gox/e2ee/types/query.pb.gois excluded by!**/*.pb.gox/e2ee/types/tx.pb.gois 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/keepertogithub.com/crypto-org-chain/cronos/x/cronos/keepercorrectly 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/v2suffix. This resolves the reported Go module import error.x/cronos/keeper/msg_server_test.go (1)
4-5: LGTM — all/v2references removed and imports updated.Go imports, go.mod, and
.protofiles no longer reference/v2.x/cronos/keeper/evmhandlers/send_to_ibc.go (1)
7-8: LGTM — All/v2import references removed. Verified no remaining/v2paths 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
cronoskeeperalias.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
v2alias 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
cronosmoduletypesalias.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
cronosprecompilesalias.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_packageoption 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_packageoption change aligns with the broader module-path refactor fromcronos/v2tocronos. 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/typesinstead 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/v2tocronos(non-v2) for:
cronoseventspackageprecompile/relayerbindings packagetypespackageThis 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_packageoption change aligns with the module-path refactor fromcronos/v2tocronos. 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_packageoption change aligns with the module-path refactor fromcronos/v2tocronos. 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
SIMAPPvariable has been updated fromgithub.com/crypto-org-chain/cronos/v2/apptogithub.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-breakingtarget failure is expected because the PR intentionally introduces breaking changes to protogo_packageoptions (module path migration fromv2to 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
--againstflag 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_packageoption change aligns with the module-path refactor fromcronos/v2tocronosfor 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_packageoption change aligns with the module-path refactor fromcronos/v2tocronosfor 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 |
randy-cro
left a comment
randy-cro
left a comment
There was a problem hiding this comment.
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.
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/v2togithub.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.mdfor 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/v2togithub.com/crypto-org-chain/cronosis correct and aligns with Go semantic versioning conventions (non-v2 modules use no/vNsuffix).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.xtag (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/v2togithub.com/crypto-org-chain/cronos.- Deprecation notice: Consider adding a deprecation notice for the old
/v2module 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
4 participants
👮🏻👮🏻👮🏻 !!!! 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