refactor(devgas): For #1866 #2199
Conversation
📝 WalkthroughWalkthroughThis pull request refactors the devgas module by replacing versioned and “types”‑suffixed import paths with a unified, non‑versioned module name. Changes include updating import paths, package declarations, and type references (e.g., FeeShare, ModuleParams, errors, and events) across application, client, proto, keeper, and simulation files. The devgasmodule subpackage is also renamed accordingly. No functional logic or control flow has been altered; the modifications are purely structural to streamline namespace usage and improve code clarity. Changes
Assessment against linked issues
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (4)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
x/devgas/v1/types/params_legacy.go (1)
3-3: Consider tracking the TODO comment.The TODO indicates this file will be removed after v0.47.x upgrade. Consider creating an issue to track this cleanup task.
Would you like me to help create an issue to track the removal of this legacy code after the v0.47.x upgrade?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
x/devgas/v1/ante/ante.go(6 hunks)x/devgas/v1/ante/ante_test.go(7 hunks)x/devgas/v1/ante/expected_keepers.go(2 hunks)x/devgas/v1/exported/exported.go(1 hunks)x/devgas/v1/genesis_test.go(8 hunks)x/devgas/v1/keeper/grpc_query_test.go(7 hunks)x/devgas/v1/keeper/keeper.go(5 hunks)x/devgas/v1/keeper/keeper_test.go(4 hunks)x/devgas/v1/keeper/msg_server.go(3 hunks)x/devgas/v1/keeper/msg_server_test.go(5 hunks)x/devgas/v1/keeper/store.go(1 hunks)x/devgas/v1/module_test.go(2 hunks)x/devgas/v1/types/expected_keepers.go(2 hunks)x/devgas/v1/types/params_legacy.go(2 hunks)x/devgas/v1/types/params_legacy_test.go(1 hunks)
✅ Files skipped from review due to trivial changes (6)
- x/devgas/v1/exported/exported.go
- x/devgas/v1/ante/expected_keepers.go
- x/devgas/v1/ante/ante.go
- x/devgas/v1/keeper/msg_server.go
- x/devgas/v1/keeper/keeper_test.go
- x/devgas/v1/keeper/grpc_query_test.go
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: e2e-evm
- GitHub Check: integration-tests
🔇 Additional comments (13)
x/devgas/v1/types/expected_keepers.go (2)
4-4: LGTM! Import aliases are now more concise and intuitive.The changes align with the PR's goal of standardizing naming conventions across the codebase.
Also applies to: 8-8
14-14: LGTM! Interface type references correctly updated.The changes consistently reflect the new import aliases while maintaining the same functionality.
Also applies to: 17-17, 31-31
x/devgas/v1/module_test.go (1)
7-8: LGTM! The import refactoring improves code clarity.The changes consistently use
devgasv1for module-level operations anddevgasfor type references, making the code more maintainable.Also applies to: 13-16, 28-29
x/devgas/v1/keeper/store.go (1)
6-8: LGTM! The type reference updates are consistent.The changes uniformly update all type references from
devgastypestodevgas, improving code clarity while maintaining functionality.Also applies to: 16-16, 22-22, 25-26, 33-33, 35-35, 37-39, 41-41, 44-44, 48-48, 50-50, 54-54
x/devgas/v1/keeper/keeper.go (1)
11-11: LGTM! The keeper refactoring is consistent.The changes uniformly update all type references from
devgastypestodevgasand correctly update the store package import, improving code clarity while maintaining functionality.Also applies to: 16-16, 22-22, 25-25, 27-27, 42-42, 44-44, 53-53, 55-55, 57-57, 71-72, 84-84
x/devgas/v1/genesis_test.go (1)
15-16: LGTM! The test refactoring is consistent.The changes uniformly update all type references from
devgastypestodevgasand correctly introduce thedevgasv1alias for module-level operations, improving code clarity while maintaining test functionality.Also applies to: 25-25, 38-38, 45-45, 55-56, 58-58, 66-67, 77-78, 88-89, 94-94, 106-106, 117-117, 121-121, 127-127
x/devgas/v1/ante/ante_test.go (1)
9-21: LGTM! Import aliases have been consistently renamed.The changes improve readability by using shorter, more intuitive aliases while maintaining the same functionality.
x/devgas/v1/keeper/msg_server_test.go (1)
8-16: LGTM! Import aliases have been consistently renamed.The changes align with the module's naming conventions and improve code readability while maintaining the same functionality.
x/devgas/v1/types/params_legacy.go (3)
7-7: LGTM!The import alias change from
paramtypestoparamsaligns with the standardization effort.
23-24: LGTM!The function signature and implementation correctly use the new
paramsalias.
28-33: LGTM!The method signature and implementation correctly use the new
paramsalias.x/devgas/v1/types/params_legacy_test.go (2)
9-9: LGTM!The import alias change is consistent with the main file.
13-13: LGTM!The type assertion correctly uses the new
paramsalias.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2199 +/- ##
=======================================
Coverage 64.78% 64.78%
=======================================
Files 277 277
Lines 22281 22281
=======================================
Hits 14435 14435
Misses 6861 6861
Partials 985 985
|
There was a problem hiding this comment.
The issue #1866 is less about renaming imports and more so suggesting to change the actual package names at the top of the Go files.
Example: EVM Module
- The module.go and genesis.go are inside
evm/evmmodulewith package nameevmmodule - The "types" inside of
evmwith package nameevm.
Contrast this with the Devgas module:
- The module.go and genesis.go are inside
devgas/v1with package namedevgas. - The types are inside of
devgas/v1/typeswith package nametypes
To make the imports similar to the EVM one, we will need to update the .proto files in devgas to export to the newly desired directory and change the package types statements to package devgas.
expertdicer
force-pushed
the
expertdicer/devgas-refactor
branch
from
cd1df04 to
7e5aef3
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
x/devgas/params_test.go (1)
10-13: 🛠️ Refactor suggestionRemove commented-out test code.
The commented-out
TestParamKeyTablefunction should either be removed entirely or restored if it's still needed. Commented-out code can lead to confusion and maintenance issues.-// func TestParamKeyTable(t *testing.T) { -// require.IsType(t, paramtypes.KeyTable{}, ParamKeyTable()) -// require.NotEmpty(t, ParamKeyTable()) -// }
🧹 Nitpick comments (1)
proto/nibiru/devgas/v1/query.proto (1)
42-46: Track pending pagination features.The TODO comments indicate pending features for pagination implementation:
- Re-implement the paginated version
- Add automatic pagination generation
Would you like me to create issues to track these pending features? I can help draft detailed issue descriptions with implementation guidelines.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (8)
x/devgas/devgas.pb.gois excluded by!**/*.pb.gox/devgas/event.pb.gois excluded by!**/*.pb.gox/devgas/genesis.pb.gois excluded by!**/*.pb.gox/devgas/keeper/testdata/reflect.wasmis excluded by!**/*.wasmx/devgas/query.pb.gois excluded by!**/*.pb.gox/devgas/query.pb.gw.gois excluded by!**/*.pb.gw.gox/devgas/tx.pb.gois excluded by!**/*.pb.gox/devgas/tx.pb.gw.gois excluded by!**/*.pb.gw.go
📒 Files selected for processing (50)
app/ante.go(1 hunks)app/ante/handler_opts.go(1 hunks)app/keepers.go(7 hunks)app/keepers/all_keepers.go(1 hunks)app/simapp/sim_test.go(2 hunks)app/wasmext/stargate_query.go(1 hunks)app/wasmext/stargate_query_test.go(1 hunks)gosdk/querier.go(1 hunks)proto/nibiru/devgas/v1/devgas.proto(1 hunks)proto/nibiru/devgas/v1/event.proto(1 hunks)proto/nibiru/devgas/v1/genesis.proto(1 hunks)proto/nibiru/devgas/v1/query.proto(1 hunks)proto/nibiru/devgas/v1/tx.proto(1 hunks)x/devgas/ante/ante.go(5 hunks)x/devgas/ante/ante_test.go(6 hunks)x/devgas/ante/expected_keepers.go(2 hunks)x/devgas/client/cli/cli_test.go(2 hunks)x/devgas/client/cli/query.go(5 hunks)x/devgas/client/cli/tx.go(4 hunks)x/devgas/codec.go(1 hunks)x/devgas/codec_test.go(1 hunks)x/devgas/devgas.go(1 hunks)x/devgas/devgas_test.go(1 hunks)x/devgas/devgasmodule/genesis.go(2 hunks)x/devgas/devgasmodule/genesis_test.go(9 hunks)x/devgas/devgasmodule/module.go(7 hunks)x/devgas/devgasmodule/module_test.go(2 hunks)x/devgas/errors.go(1 hunks)x/devgas/expected_keepers.go(1 hunks)x/devgas/export.go(1 hunks)x/devgas/genesis.go(1 hunks)x/devgas/genesis_test.go(1 hunks)x/devgas/keeper/feeshare.go(2 hunks)x/devgas/keeper/grpc_query.go(4 hunks)x/devgas/keeper/grpc_query_test.go(7 hunks)x/devgas/keeper/keeper.go(6 hunks)x/devgas/keeper/keeper_test.go(3 hunks)x/devgas/keeper/msg_server.go(13 hunks)x/devgas/keeper/msg_server_test.go(13 hunks)x/devgas/keeper/params.go(1 hunks)x/devgas/keeper/store.go(1 hunks)x/devgas/keys.go(1 hunks)x/devgas/msg.go(1 hunks)x/devgas/msg_test.go(1 hunks)x/devgas/params.go(1 hunks)x/devgas/params_legacy.go(1 hunks)x/devgas/params_legacy_test.go(1 hunks)x/devgas/params_test.go(1 hunks)x/devgas/query.go(1 hunks)x/devgas/simulation/genesis.go(2 hunks)
✅ Files skipped from review due to trivial changes (18)
- x/devgas/export.go
- x/devgas/codec.go
- x/devgas/params_legacy_test.go
- x/devgas/genesis.go
- x/devgas/params.go
- x/devgas/expected_keepers.go
- x/devgas/query.go
- x/devgas/devgas.go
- app/keepers/all_keepers.go
- x/devgas/msg_test.go
- x/devgas/genesis_test.go
- x/devgas/keys.go
- x/devgas/devgas_test.go
- x/devgas/codec_test.go
- app/ante/handler_opts.go
- app/ante.go
- x/devgas/errors.go
- x/devgas/msg.go
🧰 Additional context used
🪛 GitHub Check: break-check
proto/nibiru/devgas/v1/devgas.proto
[failure] 4-4:
File option "go_package" changed from "github.com/NibiruChain/nibiru/v2/x/devgas/v1/types" to "github.com/NibiruChain/nibiru/v2/x/devgas".
proto/nibiru/devgas/v1/event.proto
[failure] 4-4:
File option "go_package" changed from "github.com/NibiruChain/nibiru/v2/x/devgas/v1/types" to "github.com/NibiruChain/nibiru/v2/x/devgas".
proto/nibiru/devgas/v1/genesis.proto
[failure] 6-6:
File option "go_package" changed from "github.com/NibiruChain/nibiru/v2/x/devgas/v1/types" to "github.com/NibiruChain/nibiru/v2/x/devgas".
proto/nibiru/devgas/v1/query.proto
[failure] 10-10:
File option "go_package" changed from "github.com/NibiruChain/nibiru/v2/x/devgas/v1/types" to "github.com/NibiruChain/nibiru/v2/x/devgas".
proto/nibiru/devgas/v1/tx.proto
[failure] 11-11:
File option "go_package" changed from "github.com/NibiruChain/nibiru/v2/x/devgas/v1/types" to "github.com/NibiruChain/nibiru/v2/x/devgas".
🪛 GitHub Check: codecov/patch
x/devgas/client/cli/query.go
[warning] 19-20: x/devgas/client/cli/query.go#L19-L20
Added lines #L19 - L20 were not covered by tests
[warning] 45-45: x/devgas/client/cli/query.go#L45
Added line #L45 was not covered by tests
[warning] 53-53: x/devgas/client/cli/query.go#L53
Added line #L53 was not covered by tests
[warning] 56-56: x/devgas/client/cli/query.go#L56
Added line #L56 was not covered by tests
[warning] 92-92: x/devgas/client/cli/query.go#L92
Added line #L92 was not covered by tests
[warning] 94-94: x/devgas/client/cli/query.go#L94
Added line #L94 was not covered by tests
[warning] 124-124: x/devgas/client/cli/query.go#L124
Added line #L124 was not covered by tests
[warning] 126-126: x/devgas/client/cli/query.go#L126
Added line #L126 was not covered by tests
[warning] 158-158: x/devgas/client/cli/query.go#L158
Added line #L158 was not covered by tests
[warning] 160-160: x/devgas/client/cli/query.go#L160
Added line #L160 was not covered by tests
x/devgas/ante/ante.go
[warning] 80-80: x/devgas/ante/ante.go#L80
Added line #L80 was not covered by tests
x/devgas/keeper/msg_server.go
[warning] 117-117: x/devgas/keeper/msg_server.go#L117
Added line #L117 was not covered by tests
[warning] 128-128: x/devgas/keeper/msg_server.go#L128
Added line #L128 was not covered by tests
[warning] 202-202: x/devgas/keeper/msg_server.go#L202
Added line #L202 was not covered by tests
[warning] 263-263: x/devgas/keeper/msg_server.go#L263
Added line #L263 was not covered by tests
[warning] 273-273: x/devgas/keeper/msg_server.go#L273
Added line #L273 was not covered by tests
x/devgas/devgasmodule/module.go
[warning] 75-75: x/devgas/devgasmodule/module.go#L75
Added line #L75 was not covered by tests
[warning] 86-86: x/devgas/devgasmodule/module.go#L86
Added line #L86 was not covered by tests
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: e2e-evm
- GitHub Check: integration-tests
🔇 Additional comments (34)
x/devgas/keeper/params.go (1)
6-6: LGTM! Import path and return type updates align with refactoring goals.The changes correctly update the import path to use non-versioned module name and update the return type accordingly, while preserving the existing functionality.
Also applies to: 10-10
x/devgas/ante/expected_keepers.go (1)
8-8: LGTM! Interface updates align with refactoring goals.The changes correctly update the import path and interface method signatures to use non-versioned module name, while preserving the interface contract.
Also applies to: 23-24
x/devgas/devgasmodule/genesis.go (1)
1-1: LGTM! Package rename and type updates align with refactoring goals.The changes correctly:
- Rename package to avoid naming conflicts
- Update import paths to use non-versioned module name
- Update parameter and return types accordingly
while preserving the existing genesis functionality.Also applies to: 8-9, 16-16, 31-32
x/devgas/keeper/feeshare.go (1)
6-6: LGTM! Import path and type updates align with refactoring goals.The changes correctly update the import path to use non-versioned module name and update the parameter and return types accordingly, while preserving the existing feeshare functionality.
Also applies to: 13-13, 25-25
x/devgas/simulation/genesis.go (1)
14-14: LGTM! Import path and type references updated correctly.The changes align with the PR's goal of consolidating under a unified module name. The import path has been updated to use the non-versioned devgas package, and all type references have been updated accordingly.
Also applies to: 32-34, 38-38, 46-46
x/devgas/params_legacy.go (3)
1-1: LGTM! Package name updated correctly.The package name change from
typestodevgasaligns with the PR's goal of consolidating under a unified module name.
14-15: Good improvement in DefaultAllowedDenoms initialization.Changing from
[]string(nil)to[]string{}makes the intent more explicit. Both allow all denominations, but an empty slice is more idiomatic and clearer than a nil slice.
3-3: Verify the timeline for removing this legacy code.The TODO comment indicates this file should be removed after v0.47.x (v16) upgrade.
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Check for any references to v0.47.x or v16 in the codebase rg -i "v0\.47\.x|v16" --type mdLength of output: 33
Let's broaden our search to verify the removal timeline details across the entire repository instead of only Markdown files. The previous command didn't return any output likely because it was limited by file type. We'll now run a repository‐wide search for "v0.47.x" or "v16" to capture any additional contextual references.
🏁 Script executed:
#!/bin/bash # Search for references to v0.47.x or v16 across the entire codebase rg -i "v0\.47\.x|v16"Length of output: 1555
Legacy code removal timeline confirmed
The comment in
x/devgas/params_legacy.goclearly indicates that the legacy file should be removed after the v0.47.x (v16) upgrade. A repository-wide search also revealed related mentions (e.g., ingo.mod) that align with this timeline. No conflicting version references or additional timeline implications were found.x/devgas/devgasmodule/module_test.go (1)
1-1: LGTM! Package name, imports, and type references updated correctly.The changes consistently align with the PR's goal:
- Package name changed to reflect new module structure
- Import paths updated to use non-versioned packages
- Type references updated to use devgas package directly
Also applies to: 7-8, 13-16, 28-28
gosdk/querier.go (1)
9-9: LGTM! Import path updated correctly.The import path has been updated from the versioned path (
v1/types) to the non-versioned path, which aligns with the PR's goal of consolidating under a unified module name.x/devgas/keeper/store.go (1)
8-8: LGTM! Clean and consistent package refactoring.The changes consistently replace all references to
devgastypeswithdevgas, including:
- Import statements
- Type references in struct fields and method signatures
- Namespace constants
- Function parameters and return types
The refactoring maintains the original functionality while improving code organization.
Also applies to: 16-16, 22-22, 25-26, 33-33, 35-35, 37-39, 41-41, 44-44, 48-48, 50-50, 54-54
x/devgas/keeper/keeper_test.go (1)
15-16: LGTM! Test suite updated correctly.The test suite has been properly updated to use the new
devgaspackage, maintaining test coverage while aligning with the refactoring changes.Also applies to: 33-34, 46-47, 50-50
x/devgas/keeper/keeper.go (1)
16-16: LGTM! Keeper implementation updated consistently.The Keeper implementation has been properly updated to use the new
devgaspackage, including:
- Interface types for BankKeeper and AccountKeeper
- Store types and module parameters
- Logger configuration
The changes maintain the original functionality while improving code organization.
Also applies to: 25-25, 27-27, 42-42, 44-44, 55-55, 57-57, 71-72, 84-84
x/devgas/keeper/grpc_query.go (1)
11-11: LGTM! GRPC query implementation updated properly.The GRPC query implementation has been correctly updated to use the new
devgaspackage, including:
- Query server interface assertion
- Request and response types
- Parameter and return types for all query methods
The changes maintain the original functionality while improving code organization.
Also applies to: 14-14, 32-33, 39-39, 48-49, 74-74, 80-81, 84-84, 90-91, 97-97
x/devgas/devgasmodule/genesis_test.go (1)
1-132: LGTM!The package renaming and import path updates are consistent with the refactoring goals. The changes maintain the correct functionality while improving code organization.
x/devgas/client/cli/tx.go (1)
1-147: LGTM!The import path updates and message type reference changes are consistent with the refactoring goals. The changes maintain the correct functionality while improving code organization.
app/wasmext/stargate_query_test.go (1)
1-117: LGTM!The import path update is consistent with the refactoring goals. The changes maintain the correct functionality while improving code organization.
x/devgas/keeper/grpc_query_test.go (1)
10-11: LGTM! Import paths updated correctly.The changes align with the refactoring goal of streamlining the devgas module's imports.
x/devgas/ante/ante.go (1)
12-12: LGTM! Error handling and module parameters updated correctly.The changes maintain consistent error handling while streamlining the module's type system.
Also applies to: 80-80, 84-84, 95-95, 114-114, 124-124
x/devgas/client/cli/cli_test.go (1)
24-25: LGTM! Test configuration updated correctly.The changes properly update the test setup to use the new module structure while maintaining test coverage.
Also applies to: 46-46
x/devgas/devgasmodule/module.go (2)
1-1: LGTM! Package and import paths updated correctly.The package has been renamed from
devgastodevgasmodule, and import paths have been updated to use non-versioned paths, which aligns with the refactoring goals.Also applies to: 19-23
74-76: Add test coverage for error handling.These error handling cases in
ValidateGenesisandRegisterGRPCGatewayRoutesare not covered by tests.Consider adding test cases for:
- Genesis state unmarshal failure
- GRPC gateway registration failure
Also applies to: 86-88
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 75-75: x/devgas/devgasmodule/module.go#L75
Added line #L75 was not covered by testsapp/wasmext/stargate_query.go (1)
6-6: LGTM! Import path updated correctly.The import path has been updated from versioned to non-versioned, which aligns with the refactoring goals.
x/devgas/ante/ante_test.go (1)
20-21: LGTM! Import paths and type references updated correctly.The import paths have been updated to use non-versioned paths, and type references have been updated from
devgastypestodevgas, maintaining test functionality.Also applies to: 147-149
x/devgas/keeper/msg_server.go (2)
12-15: LGTM! Import paths and interface implementation updated correctly.The import paths have been updated, and the
MsgServerinterface implementation has been correctly updated to use the new package.
117-117: Add test coverage for error handling paths.Several error handling paths in the message server implementation lack test coverage:
- Fee share disabled error
- Fee share already registered error
- Fee share contract not registered error
Consider adding test cases for these error conditions to ensure robust error handling.
Also applies to: 128-128, 202-202, 263-263, 273-273
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 117-117: x/devgas/keeper/msg_server.go#L117
Added line #L117 was not covered by testsx/devgas/keeper/msg_server_test.go (1)
1-466: LGTM!The test cases are comprehensive and well-structured, covering various scenarios for fee share registration, updates, and cancellations. The test assertions are appropriate for each test case.
app/simapp/sim_test.go (1)
41-42: LGTM!The changes correctly update the import path and store key reference for the devgas module, maintaining consistency with the refactoring goal.
Also applies to: 292-293
app/keepers.go (1)
114-116: LGTM!The changes correctly update the import paths and module references for the devgas module while maintaining the existing initialization logic and module ordering.
Also applies to: 511-511, 678-680, 748-749, 854-855, 907-907
proto/nibiru/devgas/v1/devgas.proto (1)
4-4: LGTM!The go_package option update aligns with the refactoring goal of moving away from versioned paths while maintaining the existing protobuf message definitions.
🧰 Tools
🪛 GitHub Check: break-check
[failure] 4-4:
File option "go_package" changed from "github.com/NibiruChain/nibiru/v2/x/devgas/v1/types" to "github.com/NibiruChain/nibiru/v2/x/devgas".proto/nibiru/devgas/v1/genesis.proto (1)
6-6: Verify the impact of package path changes.The change in
go_packageoption from"github.com/NibiruChain/nibiru/v2/x/devgas/v1/types"to"github.com/NibiruChain/nibiru/v2/x/devgas"is a breaking change that affects import paths.❌ Verification inconclusive
🏁 Script executed:
#!/bin/bash # Search for any remaining references to the old import path rg "github.com/NibiruChain/nibiru/v2/x/devgas/v1/types" -lLength of output: 58
Impact of Package Path Change – Manual Check Recommended
The recent grep searches did not reveal any internal references to the old package path ("github.com/NibiruChain/nibiru/v2/x/devgas/v1/types"). This indicates that within the repository, the change in the
go_packageoption is isolated. However, since the change is breaking for downstream consumers who rely on the import path, please manually verify that all external dependencies (e.g., client code or other modules) have been updated to use the new package path.🧰 Tools
🪛 GitHub Check: break-check
[failure] 6-6:
File option "go_package" changed from "github.com/NibiruChain/nibiru/v2/x/devgas/v1/types" to "github.com/NibiruChain/nibiru/v2/x/devgas".proto/nibiru/devgas/v1/event.proto (1)
4-4: Package path change aligns with module refactoring.The update to
go_packageoption is consistent with the broader refactoring effort to streamline package organization.🧰 Tools
🪛 GitHub Check: break-check
[failure] 4-4:
File option "go_package" changed from "github.com/NibiruChain/nibiru/v2/x/devgas/v1/types" to "github.com/NibiruChain/nibiru/v2/x/devgas".proto/nibiru/devgas/v1/tx.proto (1)
11-11: Package path change maintains consistency.The modification to
go_packageoption maintains consistency with other proto files in the module.🧰 Tools
🪛 GitHub Check: break-check
[failure] 11-11:
File option "go_package" changed from "github.com/NibiruChain/nibiru/v2/x/devgas/v1/types" to "github.com/NibiruChain/nibiru/v2/x/devgas".proto/nibiru/devgas/v1/query.proto (1)
10-10: Package path change follows module-wide refactoring.The update to
go_packageoption aligns with the module-wide refactoring pattern.🧰 Tools
🪛 GitHub Check: break-check
[failure] 10-10:
File option "go_package" changed from "github.com/NibiruChain/nibiru/v2/x/devgas/v1/types" to "github.com/NibiruChain/nibiru/v2/x/devgas".
| @@ -10,14 +10,14 @@ | |||
| "github.com/cosmos/cosmos-sdk/client/flags" | |||
| "github.com/cosmos/cosmos-sdk/version" | |||
|
|
|||
| "github.com/NibiruChain/nibiru/v2/x/devgas/v1/types" | |||
| "github.com/NibiruChain/nibiru/v2/x/devgas" | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Add test coverage for CLI query commands.
Several modified lines in the query commands lack test coverage. Consider adding test cases to ensure the refactored code paths are properly tested.
Run this script to identify which query commands need additional test coverage:
Also applies to: 19-20, 45-45, 53-53, 56-56, 92-92, 94-94, 124-124, 126-126, 158-158, 160-160
🏁 Script executed:
#!/bin/bash
# Description: Check test coverage for CLI query commands
# Find all test files for CLI commands
fd -e go -E '*_test.go' . | grep 'cli'
# Check test coverage
go test -coverprofile=coverage.out ./x/devgas/client/cli/
go tool cover -func=coverage.out | grep -E "query.go.*statements"Length of output: 9661
Insufficient Test Coverage for CLI Query Commands
The executed scripts confirm that the CLI commands package currently has only 42.2% statement coverage. This indicates that several modified paths in the query commands—specifically those around lines 13, 19–20, 45, 53, 56, 92, 94, 124, 126, 158, and 160 in x/devgas/client/cli/query.go—are not adequately exercised by tests. Please add targeted test cases to ensure these code paths are properly verified.
I made some changes based on your guide. Can you review again? Tysm |
There was a problem hiding this comment.
The golang package changes look fine to me, but I wonder if the proto package changes will change the byte representation of serialized protos.
Can you test this change by running a localnet with latest main, and then stopping the network, hot-swapping to a binary produced by this branch, and starting the network again? I'd like to see if there are any state issues.
| @@ -445,43 +445,43 @@ func init() { | |||
| func init() { proto.RegisterFile("nibiru/devgas/v1/tx.proto", fileDescriptor_72949c99a02cd615) } | |||
|
|
|||
| var fileDescriptor_72949c99a02cd615 = []byte{ | |||
| // 566 bytes of a gzipped FileDescriptorProto | |||
There was a problem hiding this comment.
I wonder if this will break the existing state.
There was a problem hiding this comment.
It still produces the blocks well.
There was a problem hiding this comment.
I wonder if this will break the existing state.
i believe it will not break the existing state.
Purpose / Abstract