-
Notifications
You must be signed in to change notification settings - Fork 237
extract creating encoding config from initia app #385
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
base: main
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughThis pull request refactors the application’s initialization process. The changes remove deprecated and unused import statements along with legacy codec registrations. A new implementation of the encoding configuration is introduced in favor of the previous temporary app instance method. Simultaneously, the basic module manager initialization is refactored by replacing the old Changes
Sequence Diagram(s)sequenceDiagram
participant App as NewInitiaApp
participant Enc as MakeEncodingConfig
participant BM as NewBasicManager
participant Gen as NewDefaultGenesisState
participant RC as NewRootCmd
App ->> Enc: Initialize encoding configuration
App ->> BM: Set up basic module manager
Gen ->> BM: Generate default genesis state
RC ->> BM: Configure root command components
Poem
🪧 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 (
|
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Files |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #385 +/- ##
==========================================
- Coverage 40.03% 40.01% -0.03%
==========================================
Files 294 294
Lines 27508 27585 +77
==========================================
+ Hits 11012 11037 +25
- Misses 14796 14848 +52
Partials 1700 1700
🚀 New features to boost your workflow:
|
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 (2)
app/encoding.go (2)
34-37
: Function signature improvement with specific parameter.The
AutoCliOpts
function now explicitly accepts the encoding configuration as a parameter rather than creating it internally, which improves modularity. However, the map initializations could be optimized.- appModules := make(map[string]appmodule.AppModule, 0) - moduleOptions := make(map[string]interface{}, 0) + appModules := make(map[string]appmodule.AppModule) + moduleOptions := make(map[string]interface{})
40-55
: Module processing logic is correct but could be more concise.The loop correctly processes modules and identifies those with AutoCLI capabilities, storing them appropriately based on their supported interfaces.
The nested interface check could be extracted to a type definition to improve code readability:
+type moduleWithAutoCliOptions interface { + AutoCLIOptions() *autocliv1.ModuleOptions +} for _, m := range modules { if moduleWithName, ok := m.(module.HasName); ok { moduleName := moduleWithName.Name() - if _, ok := m.(interface { - AutoCLIOptions() *autocliv1.ModuleOptions - }); ok { + if _, ok := m.(moduleWithAutoCliOptions); ok { moduleOptions[moduleName] = m } else { continue }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Lite
📒 Files selected for processing (3)
app/encoding.go
(1 hunks)app/modules.go
(5 hunks)cmd/initiad/root.go
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- cmd/initiad/root.go
- app/modules.go
🔇 Additional comments (5)
app/encoding.go (5)
9-9
: Appropriate imports added for the refactored implementation.The added imports for
std
,autocliv1
, andcryptocodec
properly support the new direct codec registration approach.Also applies to: 14-14, 17-17
21-27
: Clean implementation of encoding configuration creation.The encoding configuration now directly leverages
params.MakeEncodingConfig()
and properly registers legacy Amino codecs and interfaces for both standard Cosmos SDK types and custom crypto types. This approach eliminates the previous need for a temporary application instance.
28-30
: Appropriate use of the new BasicManager.The code correctly uses the new
NewBasicManager()
function to register interfaces and legacy Amino codecs with the encoding configuration, which aligns with the broader refactoring of module management.
38-39
: Correct use of modulesForAutoCli function.The code properly retrieves modules with all necessary components for AutoCLI functionality.
58-59
: Properly updated AppOptions return value.The function now correctly returns AppOptions with
appModules
instead of the previously usedmodules
and properly extracts module options from the new map.
No description provided.