fix: remove additional log line when exporting genesis #3038
Conversation
kingpinXD
requested review from
brewmaster012,
lumtis,
ws4charlie,
skosito,
swift1337 and
fbac
as code owners
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 Walkthrough📝 Walkthrough📝 Walkthrough📝 Walkthrough📝 Walkthrough📝 Walkthrough📝 Walkthrough📝 WalkthroughWalkthroughThe changes in this pull request primarily focus on refactoring and improving the initialization processes within the ZetaChain application. Modifications include the removal of specific logging related to blocked addresses, a more structured approach to keeper initialization, and enhancements to the module management system. Additionally, the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App as ZetaChainApp
participant Keeper as VariousKeepers
participant ModuleManager as ModuleManager
User->>App: Initialize Application
App->>Keeper: Initialize Keepers
Keeper->>ModuleManager: Register Modules
ModuleManager-->>App: Modules Ready
App-->>User: Application Initialized
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
🧹 Outside diff range and nitpick comments (2)
cmd/zetacored/root.go (2)
289-300: Consider documenting magic numbers and default values.While the initialization is correct, consider:
- Adding a comment explaining why an empty map is used for skip upgrade heights
- Either documenting or extracting the hard-coded check period value (
1) into a named constant
301-306: Consider enhancing error message.While the error handling is correct, consider making the error message more descriptive by including the height value:
- return servertypes.ExportedApp{}, err + return servertypes.ExportedApp{}, fmt.Errorf("failed to load state at height %d: %w", height, err)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
- app/app.go (0 hunks)
- cmd/zetacored/root.go (1 hunks)
💤 Files with no reviewable changes (1)
- app/app.go
🧰 Additional context used
📓 Path-based instructions (1)
cmd/zetacored/root.go (1)
Pattern
**/*.go: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
🔇 Additional comments (4)
cmd/zetacored/root.go (4)
271-271: LGTM! Clear and accurate method documentation.The updated comment better reflects the method's purpose of exporting application state for genesis file creation.
277-283: LGTM! Improved variable naming and robust path validation.The variable renaming enhances code readability, and the home path validation properly handles both type assertion and empty string cases.
284-288: LGTM! Clean and focused loading logic.The introduction of the
loadLatestflag simplifies the logic and improves code readability.
308-308: LGTM! Proper state export.The export call correctly uses the initialized app instance to export the application state and validators.
|
|
||
| homePath, ok := appOpts.Get(flags.FlagHome).(string) | ||
| if !ok || homePath == "" { | ||
| return servertypes.ExportedApp{}, errors.New("application home not set") | ||
| } | ||
|
|
||
| loadLatest := false |
There was a problem hiding this comment.
Why this change in this file? looks unrelated to the purpose of the PR?
There was a problem hiding this comment.
It does not change the logic , just restructures the code to make it more readable .
I just came across this when investigating , the change seems too small to be part of a separate pr . lmk if you think otherwise
|
|
||
| homePath, ok := appOpts.Get(flags.FlagHome).(string) | ||
| if !ok || homePath == "" { | ||
| return servertypes.ExportedApp{}, errors.New("application home not set") | ||
| } | ||
|
|
||
| loadLatest := false |
Co-authored-by: Lucas Bertrand <[email protected]>
Description
appExportfunctionHow Has This Been Tested?
Summary by CodeRabbit
New Features
Bug Fixes
Refactor