-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
docs(baseapp): add instructions to change DefaultGenesis #21680
base: main
Are you sure you want to change the base?
Conversation
WalkthroughWalkthroughThe pull request introduces significant changes to the handling of the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
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
|
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: 1
Outside diff range and nitpick comments (1)
docs/build/building-apps/01-app-go-di.md (1)
176-176
: Add a blank line before the fenced code block.To adhere to the Markdown formatting best practices, add a blank line before the fenced code block. This improves the readability of the documentation.
Apply this change:
-```go + +```goTools
Markdownlint
176-176: null
Fenced code blocks should be surrounded by blank lines(MD031, blanks-around-fences)
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (1)
- docs/build/building-apps/01-app-go-di.md (1 hunks)
Additional context used
Path-based instructions (1)
docs/build/building-apps/01-app-go-di.md (1)
Pattern
**/*.md
: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
Markdownlint
docs/build/building-apps/01-app-go-di.md
176-176: null
Fenced code blocks should be surrounded by blank lines(MD031, blanks-around-fences)
Additional comments not posted (5)
docs/build/building-apps/01-app-go-di.md (5)
121-222
: Valuable addition to the documentation, but consider moving to a separate page and updating for v0.52.The new content on modifying the
DefaultGenesis
parameters for modules provides helpful insights for developers looking to customize their Cosmos SDK applications. The code examples clearly demonstrate the different approaches available.However, as mentioned in the previous review comment, this content may be better suited in a separate documentation page, rather than being included in the "Overview of
app_di.go
" section. This would help keep the documentation more focused and organized.Additionally, it's important to ensure that the documentation and code examples are up-to-date with the latest version of the Cosmos SDK (v0.52). Some concepts or APIs mentioned here may have changed or been deprecated in newer versions.
Tools
Markdownlint
176-176: null
Fenced code blocks should be surrounded by blank lines(MD031, blanks-around-fences)
129-157
: Code example for option 1 looks good.The code example demonstrating how to use a custom
CustomStakingModule
to override theDefaultGenesis
method using theModuleBasics
variable is clear and easy to understand. It shows how to wrap the existing staking module and provide a custom implementation.
159-167
: Code example for option 2 is clear and concise.The code example for option 2 demonstrates how to create a basic module manager from the existing module manager and override the staking module's
AppModuleBasic
implementation. This approach is straightforward and allows for targeted customization of specific modules.
168-174
: Code example for option 3 showcases the use ofdepinject
.The code example for option 3 illustrates how to use
depinject.Supply
to provide custom module basics. This approach leverages dependency injection to customize the behavior of specific modules. It's a clean and flexible way to extend or modify module functionality.
181-208
: Code example for customizingDefaultGenesis
using the new module manager is well-structured.The code example demonstrates how to create a
CustomStakingModule
that embeds thestaking.AppModule
and overrides theDefaultGenesis
method. It shows two options for integrating the custom module:
- Creating a new module manager with the custom staking module.
- Overriding the existing module manager's staking module with the custom one.
Both approaches are clearly explained and the code is properly structured.
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.
See #21680 (comment)
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: 1
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (1)
- docs/build/building-apps/06-app-go-genesis.md (1 hunks)
Additional context used
Path-based instructions (1)
docs/build/building-apps/06-app-go-genesis.md (1)
Pattern
**/*.md
: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
Markdownlint
docs/build/building-apps/06-app-go-genesis.md
6-6: null
First line in a file should be a top-level heading(MD041, first-line-heading, first-line-h1)
Additional comments not posted (1)
docs/build/building-apps/06-app-go-genesis.md (1)
13-53
: LGTM!The Go code example is well-structured and follows best practices. It provides a clear demonstration of how to modify the
DefaultGenesis
parameters for the staking module. The two options for overriding the module manager are also helpful for different use cases.
} | ||
|
||
|
||
// depinject users |
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.
I think the flow is backwards:
- first you inject a module manager (or have it already without depinject)
- then you modify it (option 2 for depinject users, option 1 for non depinject users)
- then you set it on the app (missing here)
&moduleManager, | ||
) | ||
|
||
// non-depinject users |
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.
this is unrelated, non depinject users always need to call this
}) | ||
} | ||
|
||
// option 1: use new module manager |
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.
I'd say this is the option for non depinject users
// other modules ... | ||
}) | ||
|
||
// option 2: override previous module manager |
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.
This one is best for depinject users
Description
Closes: #11008
Add documentation on how to change the default genesis.
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!
in the type prefix if API or client breaking changeCHANGELOG.md
Reviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
Please see Pull Request Reviewer section in the contributing guide for more information on how to review a pull request.
I have...
Summary by CodeRabbit
New Features
CustomStakingModule
for enhanced control over the Staking module's behavior.BondDenom
.Improvements
These changes enhance the modularity and configurability of the Cosmos SDK application framework, making it easier for developers to tailor module behaviors.