docs: rewrite building module section (1/n) - mm #22724
Conversation
julienrbrt
added
the
backport/v0.52.x
📝 Walkthrough📝 WalkthroughWalkthroughThis pull request introduces significant changes to the Cosmos SDK documentation, including the removal of the Changes
Possibly related PRs
Suggested reviewers
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: 1
🧹 Outside diff range and nitpick comments (5)
docs/build/building-modules/00-intro.md (1)
45-45: Consider adding security implications of super-user capabilities.While the explanation of module capabilities is accurate, it would be beneficial to explicitly document the security implications and best practices when implementing these privileged functions.
docs/build/building-modules/01-module-manager.md (2)
21-24: Add migration guidance between appmodule versions.While the distinction between appmodule and appmodule v2 is clear, consider adding:
- Migration steps for upgrading from v1 to v2
- Compatibility considerations
- Best practices for choosing between versions in new modules
🧰 Tools
🪛 LanguageTool
[grammar] ~21-~21: You should probably use “is”.
Context: ...osmos SDK application. Those interface are defined in the `cosmossdk.io/core/appmo...(SINGULAR_AGREEMENT_SENT_START)
46-48: Enhance deprecation notice with timeline.The legacy interface documentation would benefit from:
- A specific timeline or version for the planned removal
- Links to migration guides
- Examples of modern alternatives
🧰 Tools
🪛 LanguageTool
[style] ~46-~46: ‘Prior to’ might be wordy. Consider a shorter alternative.
Context: ... placeholder functions. :::note legacy Prior to the introduction of the `cosmossdk.io/c...(EN_WORDINESS_PREMIUM_PRIOR_TO)
docs/learn/beginner/00-app-anatomy.md (1)
60-62: Track TODO comment in issue tracker.The TODO comment about introducing runtime documentation should be tracked in the issue system for proper follow-up.
Would you like me to help create a GitHub issue to track this documentation task?
docs/build/building-apps/00-runtime.md (1)
7-9: Consider enhancing the documentation with examplesWhile the current documentation provides a good introduction, it would be more helpful to include:
- A code snippet demonstrating how to instantiate and configure
runtime.App- Links to API references or related documentation
- Common usage patterns or best practices
Would you like me to help generate example code snippets and additional documentation sections?
🧰 Tools
🪛 LanguageTool
[misspelling] ~8-~8: Use “A” instead of ‘An’ if the following word doesn’t start with a vowel sound, e.g. ‘a sentence’, ‘a university’.
Context: ...lications, the codecs, and the stores. An user only needs to importruntimein ...(EN_A_VS_AN)
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
📒 Files selected for processing (5)
docs/build/building-apps/00-app-go.md(0 hunks)docs/build/building-apps/00-runtime.md(1 hunks)docs/build/building-modules/00-intro.md(2 hunks)docs/build/building-modules/01-module-manager.md(3 hunks)docs/learn/beginner/00-app-anatomy.md(2 hunks)
💤 Files with no reviewable changes (1)
- docs/build/building-apps/00-app-go.md
🧰 Additional context used
📓 Path-based instructions (4)
docs/build/building-modules/00-intro.md (1)
Pattern **/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
docs/learn/beginner/00-app-anatomy.md (1)
Pattern **/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
docs/build/building-modules/01-module-manager.md (1)
Pattern **/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
docs/build/building-apps/00-runtime.md (1)
Pattern **/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
🪛 LanguageTool
docs/build/building-modules/01-module-manager.md
[style] ~8-~8: Consider a shorter alternative to avoid wordiness.
Context: ...faces](#application-module-interfaces), in order to be managed by the application's [module...
(IN_ORDER_TO_PREMIUM)
[style] ~8-~8: The phrase “a variety of” may be wordy. To make your writing clearer, consider replacing it.
Context: ...lopers to set the order of execution of a variety of functions like [PreBlocker, `BeginBlo...
(A_VARIETY_OF)
[grammar] ~21-~21: You should probably use “is”.
Context: ...osmos SDK application. Those interface are defined in the `cosmossdk.io/core/appmo...
(SINGULAR_AGREEMENT_SENT_START)
[uncategorized] ~30-~30: Loose punctuation mark.
Context: ...ppmodule.HasPreBlocker`](#haspreblocker): The extension interface that contains i...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~31-~31: Loose punctuation mark.
Context: ...dule.HasBeginBlocker`](#hasbeginblocker): The extension interface that contains i...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~32-~32: Loose punctuation mark.
Context: ...ppmodule.HasEndBlocker`](#hasendblocker): The extension interface that contains i...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~33-~33: Loose punctuation mark.
Context: ...module.HasABCIEndBlock`](#hasendblocker): The extension interface that contains i...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~34-~34: Loose punctuation mark.
Context: ...sterInterfaces`](#hasregisterinterfaces): The extension interface for modules to ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~35-~35: Loose punctuation mark.
Context: ...* appmodule.HasService: The extension interface for modules to ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~35-~35: A comma may be missing after the conjunctive/linking adverb ‘However’.
Context: ...sed in core to avoid a gRPC dependency. However it is usable in an application. * [`app...
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
[uncategorized] ~36-~36: Loose punctuation mark.
Context: ...ppmodule.HasAminoCodec`](#hasaminocodec): The extension interface for modules to ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~37-~37: Loose punctuation mark.
Context: ...ppmodule.HasMigrations`](#hasmigrations): The extension interface for registering...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~38-~38: Loose punctuation mark.
Context: ...ConsensusVersion`](#hasconsensusversion): The extension interface for declaring a...
(UNLIKELY_OPENING_PUNCTUATION)
[style] ~38-~38: ‘in conjunction with’ might be wordy. Consider a shorter alternative.
Context: ...sion. It is usually not used alone, but in conjunction with HasMigrations. * [`appmodule.HasGenes...
(EN_WORDINESS_PREMIUM_IN_CONJUNCTION_WITH)
[style] ~46-~46: ‘Prior to’ might be wordy. Consider a shorter alternative.
Context: ... placeholder functions. :::note legacy Prior to the introduction of the `cosmossdk.io/c...
(EN_WORDINESS_PREMIUM_PRIOR_TO)
[grammar] ~108-~108: Did you mean “registering”? Or maybe you should add a pronoun? In active voice, ‘allow’ + ‘to’ takes an object, usually a pronoun.
Context: ...interface defines one method. It allows to register and let module expose gRPC services. Th...
(ALLOW_TO)
[grammar] ~117-~117: Did you mean “registering”? Or maybe you should add a pronoun? In active voice, ‘allow’ + ‘to’ takes an object, usually a pronoun.
Context: ...AminoCodec TheHasAminoCodecallows to register theamino` codec for the module, which...
(ALLOW_TO)
[style] ~117-~117: Consider a shorter alternative to avoid wordiness.
Context: ... and unmarshal structs to/from []byte in order to persist them in the module's KVStore....
(IN_ORDER_TO_PREMIUM)
[style] ~141-~141: ‘in conjunction with’ might be wordy. Consider a shorter alternative.
Context: ...le consensus version. It is mainly used in conjunction with HasMigrations. ```go reference https...
(EN_WORDINESS_PREMIUM_IN_CONJUNCTION_WITH)
[uncategorized] ~159-~159: Loose punctuation mark.
Context: ...me of the methods: * DefaultGenesis(): Returns a default [GenesisState](./08...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~160-~160: Loose punctuation mark.
Context: ...dateGenesis(data json.RawMessage) error: Used to validate the GenesisState` def...
(UNLIKELY_OPENING_PUNCTUATION)
[grammar] ~162-~162: Comparison is written ‘the same … as’.
Context: ...the module developer. In the same vein than HasABCIEndBlock, HasABCIGenesis is ...
(THE_SAME_AS)
docs/build/building-apps/00-runtime.md
[misspelling] ~8-~8: Use “A” instead of ‘An’ if the following word doesn’t start with a vowel sound, e.g. ‘a sentence’, ‘a university’.
Context: ...lications, the codecs, and the stores. An user only needs to import runtime in ...
(EN_A_VS_AN)
🔇 Additional comments (1)
docs/build/building-modules/00-intro.md (1)
20-22: LGTM! Clear explanation of core functionalities.
The analogy with Next.js and Ruby-on-Rails effectively communicates the framework's role, and the explanation of core functionalities and runtime is accurate and well-structured.
|
|
||
| The `runtime` package is the Cosmos SDK package that combines the building blocks of your blockchain together. It wires together the modules, the applications, the codecs, and the stores. | ||
|
|
||
| An user only needs to import `runtime` in their `app.go` and instantiate a `runtime.App`. |
There was a problem hiding this comment.
Fix article usage
Replace "An user" with "A user" as the word "user" begins with a consonant sound.
-An user only needs to import `runtime` in their `app.go` and instantiate a `runtime.App`.
+A user only needs to import `runtime` in their `app.go` and instantiate a `runtime.App`.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| An user only needs to import `runtime` in their `app.go` and instantiate a `runtime.App`. | |
| A user only needs to import `runtime` in their `app.go` and instantiate a `runtime.App`. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (10)
docs/build/building-modules/06-keeper.md (2)
Line range hint
82-82: Fix broken markdown link syntaxThe link to Collections documentation uses HTML comments which breaks the markdown rendering. Consider using proper markdown link syntax instead.
-<!-- markdown-link-check-disable --> -State management is recommended to be done via [Collections](../packages/collections) -<!-- The above link is created via the script to generate docs --> +State management is recommended to be done via [Collections](../packages/collections).
Line range hint
89-89: Fix typo in package linkThere's a typo in the collections package link: "pacakges" should be "packages".
-[collections package](../pacakges/02-collections.md) +[collections package](../packages/02-collections.md)docs/build/building-modules/04-query-services.md (1)
55-55: Improve bullet point formatting for consistencyThe bullet point about gas tracking should be properly indented to match the style of other bullet points in the list.
-* it has its gas tracked, to avoid the attack vector where no gas is accounted for on potentially high-computation queries. +* The query has its gas tracked, to avoid the attack vector where no gas is accounted for on potentially high-computation queries.docs/build/building-modules/02-messages-and-queries.md (2)
54-54: Add missing comma for proper grammarAdd a comma after "custom signers" for better readability.
-If there is a need for custom signers then there is an alternative path which can be taken. +If there is a need for custom signers, then there is an alternative path which can be taken.🧰 Tools
🪛 LanguageTool
[typographical] ~54-~54: Consider adding a comma.
Context: ...` If there is a need for custom signers then there is an alternative path which can ...(IF_THEN_COMMA)
58-65: Enhance error handling documentationThe code example should include comments explaining the error handling strategy.
// Extract the signer from the signature. signer, err := coretypes.LatestSigner(Tx).Sender(ethTx) +// Return nil and error if signer extraction fails if err != nil { return nil, err } -// Return the signer in the required format. +// Return the signer bytes in the required [][]byte format for the CustomGetSigner interface return [][]byte{signer.Bytes()}, nildocs/build/building-modules/03-msg-services.md (2)
94-96: Improve warning message punctuationThe warning message about telemetry should end with a period for consistency with other documentation.
-Telemetry adds a performance overhead to the chain. It is recommended to only use this in critical paths +Telemetry adds a performance overhead to the chain. It is recommended to only use this in critical paths.
108-110: Enhance warning message clarityThe warning about baseapp vs Cosmos SDK v2 needs more context for readers to understand the distinction.
-This flow concerns only a Cosmos SDK *baseapp*, and not Cosmos SDK v2. +This flow applies only to the traditional Cosmos SDK baseapp implementation, and not to the new architecture introduced in Cosmos SDK v2.UPGRADING.md (3)
418-419: Improve formatting and documentation referenceConsider restructuring this section to make it more prominent:
- Use a bullet point for the interface renaming
- Add a separate paragraph for the new simsx framework introduction
- Make the documentation link more visible
-The interface `HasProposalMsgs` has been renamed to `HasLegacyProposalMsgs`, as we've introduced a new simulation framework, simpler and easier to use, named [simsx](https://github.com/cosmos/cosmos-sdk/blob/main/simsx/README.md). +* The interface `HasProposalMsgs` has been renamed to `HasLegacyProposalMsgs`. + +A new simulation framework, simsx, has been introduced which is simpler and easier to use. +For more information, see the [simsx documentation](https://github.com/cosmos/cosmos-sdk/blob/main/simsx/README.md).
Line range hint
1-24: Enhance document navigation and readabilityConsider adding these improvements to make the document more user-friendly:
- Add a table of contents at the beginning
- Include version-specific quick links
- Add a summary of breaking changes for each version
Line range hint
1-1000: Standardize technical terminologyThroughout the document, ensure consistent usage of technical terms:
- Use "Cosmos SDK" instead of mixing "SDK" and "Cosmos SDK"
- Standardize version references (e.g., "v0.47.x" vs "v0.47")
- Use consistent capitalization for terms like "BaseApp", "SimApp"
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
📒 Files selected for processing (6)
UPGRADING.md(1 hunks)docs/build/building-apps/00-runtime.md(1 hunks)docs/build/building-modules/02-messages-and-queries.md(4 hunks)docs/build/building-modules/03-msg-services.md(3 hunks)docs/build/building-modules/04-query-services.md(3 hunks)docs/build/building-modules/06-keeper.md(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/build/building-apps/00-runtime.md
🧰 Additional context used
📓 Path-based instructions (5)
docs/build/building-modules/04-query-services.md (1)
Pattern **/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
docs/build/building-modules/03-msg-services.md (1)
Pattern **/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
docs/build/building-modules/06-keeper.md (1)
Pattern **/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
docs/build/building-modules/02-messages-and-queries.md (1)
Pattern **/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
UPGRADING.md (1)
Pattern **/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
🪛 LanguageTool
docs/build/building-modules/02-messages-and-queries.md
[typographical] ~54-~54: Consider adding a comma.
Context: ...` If there is a need for custom signers then there is an alternative path which can ...
(IF_THEN_COMMA)
🔇 Additional comments (1)
docs/build/building-modules/06-keeper.md (1)
Line range hint 87-89: LGTM: Well-structured explanation of state management
The content provides clear and valuable guidance about state management best practices, effectively explaining:
- The importance of selective state management
- The role of Protocol Buffers
- The benefits of using the collections package
| ## Environment | ||
|
|
There was a problem hiding this comment.
Empty section needs to be addressed
The "Environment" section appears to be empty. Consider either:
- Adding the intended content for this section, or
- Removing the section header if it's not meant to be included in this PR.
There was a problem hiding this comment.
will be in a follow-up as said in the description
There was a problem hiding this comment.
@julienrbrt, understood. We'll look forward to the follow-up as mentioned in the description. Thank you!
(cherry picked from commit 5426cd8)
…22730) Co-authored-by: Julien Robert <[email protected]>
Description
ref: #21429
Focuses on the module manager and core API (expect environment)
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.mdReviewers 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
app.goto streamline resources.runtimepackage, clarifying its integration in blockchain applications.UPGRADING.mdto reflect changes for upgrading to the latest Cosmos SDK version, including nested message simulation and module management adjustments.