chore(x/accounts): use cosmossdk.io/core/codec instead of github.com/cosmos/cosmos-sdk/codec
#23313
Conversation
…cosmos/cosmos-sdk/codec
📝 WalkthroughWalkthroughThe pull request focuses on updating the codec import across multiple files in the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Finishing Touches
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
Documentation and Community
|
![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 (2)
x/accounts/internal/implementation/implementation.go (1)
Line range hint
19-24: Consider updating the interface definition.The
LegacyStateCodecinterface could be updated to use the newcodec.Codectype directly instead of defining the interface inline.- LegacyStateCodec interface { - Marshal(gogoproto.Message) ([]byte, error) - Unmarshal([]byte, gogoproto.Message) error - } + LegacyStateCodec codec.Codecx/accounts/defaults/multisig/utils_test.go (1)
Line range hint
29-126: Consider simplifying the mock implementation.While the implementation is thorough, most methods panic with "unimplemented". Consider:
- Using a more focused mock that only implements the required methods
- Adding TODO comments for methods that might be needed in future tests
type mockStateCodec struct { codec.Codec } -// GetMsgAnySigners implements codec.Codec. -func (mockStateCodec) GetMsgAnySigners(msg *types.Any) ([][]byte, protoreflect.Message, error) { - panic("unimplemented") -} -// ... (remove other unimplemented methods) // Only keep the methods that are actually used func (c mockStateCodec) Marshal(m gogoproto.Message) ([]byte, error) { if m == nil || gogoproto.Size(m) == 0 { return []byte{}, nil } return gogoproto.Marshal(m) } func (c mockStateCodec) Unmarshal(bz []byte, ptr gogoproto.Message) error { return gogoproto.Unmarshal(bz, ptr) } // TODO: Implement other methods as needed for future tests
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
x/accounts/defaults/base/utils_test.go(1 hunks)x/accounts/defaults/lockup/utils_test.go(1 hunks)x/accounts/defaults/multisig/utils_test.go(1 hunks)x/accounts/internal/implementation/implementation.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
x/accounts/defaults/base/utils_test.go (2)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern **/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"
x/accounts/defaults/lockup/utils_test.go (2)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern **/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"
x/accounts/defaults/multisig/utils_test.go (2)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern **/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"
x/accounts/internal/implementation/implementation.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: test-simapp-v2
- GitHub Check: test-system-v2
- GitHub Check: Analyze
- GitHub Check: Summary
🔇 Additional comments (6)
x/accounts/defaults/base/utils_test.go (2)
12-12: LGTM: Import change aligns with the PR objective.The transition to
cosmossdk.io/core/codecis correctly implemented.
Line range hint
27-41: LGTM: Robust implementation of mockStateCodec.The implementation correctly handles:
- Nil message cases
- Empty message cases
- Proper marshaling/unmarshaling
x/accounts/defaults/lockup/utils_test.go (2)
13-13: LGTM: Consistent import update.The import change aligns with the PR objective and maintains consistency with other files.
Line range hint
28-42: LGTM: Consistent mockStateCodec implementation.The implementation maintains consistency with other test files and properly handles edge cases.
x/accounts/internal/implementation/implementation.go (1)
12-12: LGTM: Clean import update.The transition to
cosmossdk.io/core/codecis properly implemented in this core file.x/accounts/defaults/multisig/utils_test.go (1)
15-15: LGTM: Consistent import update.The import change aligns with the PR objective and maintains consistency across the codebase.
|
seems like a few tests are failing, can you look into them |
Thanks, I'm going to check this |
|
could you please merge all your prs making the same change in other repos into this pr. It helps reduce the amount of reviews needed |
@tac0turtle Hi sir, how about combining all these PRs into three PRs? |
@caseylove just combining all into one (this PR) would be best |
Description
Partially addresses: #23253
Also, this PR is a supplementary of #23300 for test files
Summary by CodeRabbit
Dependency Updates
cosmossdk.io/core/codecCode Maintenance
mockStateCodecstruct in test files to implement new codec interface