docs: learn/advance #22367
docs: learn/advance #22367
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThe pull request includes extensive documentation updates across multiple files related to the Cosmos SDK. Key changes involve clarifying the functionality of 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: 4
🧹 Outside diff range and nitpick comments (7)
docs/learn/advanced/06-grpc_rest.md (1)
81-81: Consider improving the sentence structureWhile the link update is correct, consider rephrasing for better clarity:
-The Cosmos SDK's [Swagger generation script](https://github.com/cosmos/cosmos-sdk/blob/v0.52.0-beta.2/scripts/protoc-swagger-gen.sh) is a good place to start. +For generating Swagger definitions, you can refer to the [Swagger generation script](https://github.com/cosmos/cosmos-sdk/blob/v0.52.0-beta.2/scripts/protoc-swagger-gen.sh) in the Cosmos SDK repository.docs/learn/advanced/05-encoding.md (3)
37-37: Consider using a version-independent link for collections documentation.The link to the collections package includes a specific version (
v0.4.0). Consider using a version-independent link (e.g.,/main/branch) to ensure the documentation remains current, or add a note indicating which SDK version this documentation corresponds to.
78-78: Consider using version-independent links for code references.The code references use specific SDK version tags (v0.52.0-beta.1). Consider:
- Using version-independent links (e.g.,
/main/branch), or- Adding a note specifying which SDK version this documentation targets.
This will help maintain documentation relevance across SDK versions.
146-146: Use more common American English phrasing.For consistency with American English style:
-Have a look at [a real-life example in the x/auth module] +Take a look at [a real-life example in the x/auth module]🧰 Tools
🪛 LanguageTool
[locale-violation] ~146-~146: In American English, “take a look” is more commonly used.
Context: ...face` method to bundle these two steps. Have a look at [a real-life example in the x/auth m...(HAVE_A_LOOK)
docs/learn/advanced/07-cli.md (2)
40-40: Grammar and style improvements neededSeveral grammatical improvements would enhance readability:
- Line 40: Add a comma before 'so': "persist
[]bytes, so the developer"- Line 60: Add a comma after "credentials": "storage of key credentials, look at"
- Line 90: Add a comma after "By default": "By default, the app uses"
- Line 123: Replace "in order to" with "to" for conciseness
Also applies to: 60-60, 90-90, 123-123
🧰 Tools
🪛 LanguageTool
[uncategorized] ~40-~40: Use a comma before ‘so’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...tion - stores can only persist[]bytes so the developer must define a serializati...(COMMA_COMPOUND_SENTENCE_2)
Line range hint
171-189: Consider enhancing environment variables documentationThe environment variables section could be improved by:
- Adding more real-world examples of commonly used environment variables
- Including a note about environment variable precedence over flags
- Mentioning best practices for managing environment variables in different environments
🧰 Tools
🪛 LanguageTool
[uncategorized] ~169-~169: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...d to commands; some toggle an option off but others are empty values that the user n...(COMMA_COMPOUND_SENTENCE_2)
docs/learn/advanced/00-baseapp.md (1)
Line range hint
1013-1027: Maintain consistent version referencesThe code references for ExtendVote and VerifyVoteExtension use different SDK versions (v0.52.0-beta.2 and v0.50.0-alpha.0). For consistency, they should reference the same version.
Update the VerifyVoteExtension reference to use v0.52.0-beta.2:
-https://github.com/cosmos/cosmos-sdk/blob/v0.50.0-alpha.0/baseapp/abci_utils.go#L282-L288 +https://github.com/cosmos/cosmos-sdk/blob/v0.52.0-beta.2/baseapp/abci_utils.go#L282-L288
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
📒 Files selected for processing (7)
- docs/learn/advanced/00-baseapp.md (2 hunks)
- docs/learn/advanced/05-encoding.md (6 hunks)
- docs/learn/advanced/06-grpc_rest.md (3 hunks)
- docs/learn/advanced/07-cli.md (8 hunks)
- docs/learn/advanced/08-events.md (3 hunks)
- docs/learn/advanced/09-telemetry.md (1 hunks)
- docs/learn/advanced/11-runtx_middleware.md (3 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
docs/learn/advanced/00-baseapp.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"docs/learn/advanced/05-encoding.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"docs/learn/advanced/06-grpc_rest.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"docs/learn/advanced/07-cli.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"docs/learn/advanced/08-events.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"docs/learn/advanced/09-telemetry.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"docs/learn/advanced/11-runtx_middleware.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
🪛 LanguageTool
docs/learn/advanced/05-encoding.md
[locale-violation] ~146-~146: In American English, “take a look” is more commonly used.
Context: ...face` method to bundle these two steps. Have a look at [a real-life example in the x/auth m...(HAVE_A_LOOK)
docs/learn/advanced/07-cli.md
[uncategorized] ~40-~40: Use a comma before ‘so’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...tion - stores can only persist[]bytes so the developer must define a serializati...(COMMA_COMPOUND_SENTENCE_2)
[style] ~60-~60: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ree/v0.52.0-beta.2/client/keys/add.go). For more details about usage of `--keyring-...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~60-~60: Possible missing comma found.
Context: ...--keyring-backendfor storage of key credentials look at the [keyring docs](https://docs...(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~66-~66: Consider removing “of” to be more concise
Context: ...be run before every execution, and adds all of the necessary subcommands. ```go reference...(ALL_OF_THE)
[uncategorized] ~90-~90: Did you mean: “By default,”?
Context: .../simapp/simd/cmd/config.go#L41-L57 ``` By default the app uses CometBFT app config templa...(BY_DEFAULT_COMMA)
[style] ~123-~123: Consider a shorter alternative to avoid wordiness.
Context: ...saction requires some sort of signature in order to be valid, the signing command is necess...(IN_ORDER_TO_PREMIUM)
docs/learn/advanced/08-events.md
[misspelling] ~57-~57: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...d'| Query all transactions containing a x/bankSend[ServiceMsg`](../../bui...(EN_A_VS_AN)
docs/learn/advanced/09-telemetry.md
[uncategorized] ~22-~22: You might be missing the article “a” here.
Context: ...emitting and collecting metrics through simple [API](https://github.com/cosmos/cosmos-...(AI_EN_LECTOR_MISSING_DETERMINER_A)
docs/learn/advanced/11-runtx_middleware.md
[uncategorized] ~50-~50: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...y default, that panic would be recovered and an error message will be printed to log...(COMMA_COMPOUND_SENTENCE_2)
[typographical] ~50-~50: It seems that a comma is missing.
Context: ...ill be printed to log. To override that behaviour we should register a custom RecoveryHan...(IN_ORDER_TO_VB_COMMA)
🔇 Additional comments (11)
docs/learn/advanced/09-telemetry.md (1)
22-22: Version update looks goodThe documentation link has been correctly updated to reference the latest SDK version (v0.52.0-beta.2), which aligns with the version updates across other documentation files in this PR.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~22-~22: You might be missing the article “a” here.
Context: ...emitting and collecting metrics through simple [API](https://github.com/cosmos/cosmos-...(AI_EN_LECTOR_MISSING_DETERMINER_A)
docs/learn/advanced/08-events.md (4)
23-23: LGTM: URL reference update is accurate.The updated URL correctly points to the latest SDK version and relevant proto file for events.
68-68: LGTM: Code references are up-to-date.The code references have been correctly updated to point to the latest SDK version.
Also applies to: 76-76
79-80: LGTM: Documentation updates improve clarity.The added clarification about preferred event emission methods and deprecation notice is helpful for developers.
Also applies to: 83-83
88-88: LGTM: Code example effectively demonstrates typed events.The reference provides a clear, real-world example of typed event usage from the group keeper implementation.
docs/learn/advanced/06-grpc_rest.md (3)
29-29: LGTM: Correct relative link to encoding documentationThe reference to the encoding documentation is properly formatted and correctly uses a relative path.
69-69: Please verify the line numbers in the Protobuf referenceThe version update to
v0.52.0-beta.2is correct, but please verify that the line numbers#L23-L30still point to the correct Protobuf query definition in the referenced file.✅ Verification successful
Line numbers in the Protobuf reference are correct
The line numbers
#L23-L30correctly point to theAllBalancesquery definition in the bank module's Protobuf file. The referenced section shows the complete RPC definition including its documentation and HTTP endpoint configuration.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify the line numbers in the referenced proto file gh api repos/cosmos/cosmos-sdk/contents/x/bank/proto/cosmos/bank/v1beta1/query.proto?ref=v0.52.0-beta.2 | jq -r '.content' | base64 -d | sed -n '23,30p'Length of output: 226
Script:
#!/bin/bash # Try using curl to fetch the raw content directly curl -s "https://raw.githubusercontent.com/cosmos/cosmos-sdk/v0.52.0-beta.2/x/bank/proto/cosmos/bank/v1beta1/query.proto" | sed -n '23,30p'Length of output: 578
34-34: Please verify the line numbers in the code referenceThe version update to
v0.52.0-beta.2is correct, but please verify that the line numbers#L45-L47still point to the correct code segment in the referenced file.✅ Verification successful
Line numbers in the GitHub reference are correct
The line numbers
#L45-L47in the reference link correctly point to theRegisterTxServicecode segment in thev0.52.0-beta.2version ofserver/types/app.go. The referenced lines contain the documentation and function signature for gRPC Query service registration.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify the line numbers in the referenced file gh api repos/cosmos/cosmos-sdk/contents/server/types/app.go?ref=v0.52.0-beta.2 | jq -r '.content' | base64 -d | sed -n '45,47p'Length of output: 201
Script:
#!/bin/bash # Try a different approach to verify the content using curl curl -s -L "https://raw.githubusercontent.com/cosmos/cosmos-sdk/v0.52.0-beta.2/server/types/app.go" | sed -n '45,47p'Length of output: 269
docs/learn/advanced/07-cli.md (1)
46-46: Version references are correctly updatedAll code references have been consistently updated to v0.52.0-beta.2, maintaining documentation accuracy.
Also applies to: 60-60, 62-62, 69-69, 82-82, 88-88, 94-94, 102-102, 108-108, 118-118, 130-130, 143-143, 157-157
docs/learn/advanced/00-baseapp.md (2)
Line range hint
1-1028: Documentation quality is excellentThe documentation is well-written, clear, and maintains a professional tone throughout. The explanations of complex concepts are thorough and accessible.
Line range hint
1-1028: Documentation structure and references are well-organizedThe documentation maintains excellent cross-referencing and linking between related concepts, making it easy to navigate and understand the relationships between different components.
docs/learn/advanced/05-encoding.md
Outdated
| @@ -34,14 +34,14 @@ tree. | |||
|
|
|||
| For storage encoding, module developers are encouraged to use Protobuf encoding for their types | |||
| but may choose any encoding schema they like. The | |||
| [collections](../../build/packages/02-collections.md) package automatically handles encoding and | |||
| [collections](https://github.com/cosmos/cosmos-sdk/blob/collections/v0.4.0/collections/README.md) package automatically handles encoding and | |||
There was a problem hiding this comment.
when the docs build script this gets fixed, it can be reverted.
There was a problem hiding this comment.
it should work and resolve this
docs/learn/advanced/08-events.md
Outdated
| | `tx.height=23` | Query all transactions at height 23 | | ||
| | `message.action='/cosmos.bank.v1beta1.Msg/Send'` | Query all transactions containing an x/bank `Send` [Service `Msg`](../../build/building-modules/03-msg-services.md). Note the `'`s around the value. | | ||
| | `message.module='bank'` | Query all transactions containing messages from the x/bank module. Note the `'`s around the value. | | ||
| | `create_validator.validator='cosmosval1...'` | x/staking-specific Event, see [x/staking SPEC](../../../x/staking/README.md#events). | |
There was a problem hiding this comment.
yeah this should be reverted, it will resolve on the website, but not on here
There was a problem hiding this comment.
lgtm!, but could you revert the two links @tac0turtle pointed out?
docs/learn/advanced/08-events.md
Outdated
| | `tx.height=23` | Query all transactions at height 23 | | ||
| | `message.action='/cosmos.bank.v1beta1.Msg/Send'` | Query all transactions containing an x/bank `Send` [Service `Msg`](../../build/building-modules/03-msg-services.md). Note the `'`s around the value. | | ||
| | `message.module='bank'` | Query all transactions containing messages from the x/bank module. Note the `'`s around the value. | | ||
| | `create_validator.validator='cosmosval1...'` | x/staking-specific Event, see [x/staking SPEC](../../../x/staking/README.md#events). | |
There was a problem hiding this comment.
yeah this should be reverted, it will resolve on the website, but not on here
julienrbrt
added
the
backport/v0.52.x
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
docs/learn/advanced/11-runtx_middleware.md (1)
50-50: Consider improving sentence structure with proper punctuationThe sentence could be clearer with better punctuation:
-By default, that panic would be recovered and an error message will be printed to log. +By default, that panic would be recovered, and an error message will be printed to the log.🧰 Tools
🪛 LanguageTool
[uncategorized] ~50-~50: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...y default, that panic would be recovered and an error message will be printed to log...(COMMA_COMPOUND_SENTENCE_2)
[typographical] ~50-~50: It seems that a comma is missing.
Context: ...ill be printed to log. To override that behaviour we should register a custom RecoveryHan...(IN_ORDER_TO_VB_COMMA)
docs/learn/advanced/05-encoding.md (2)
40-43: Consider enhancing interface documentation.The explanation of
BinaryCodecandJSONCodecinterfaces could be more detailed. Consider adding:
- Concrete examples of when each interface is used
- The relationship between these interfaces
- Common implementation patterns
Would you like me to help draft the additional documentation?
146-146: Consider rephrasing for American English style.Consider changing "Have a look at" to "Take a look at" to align with American English conventions.
🧰 Tools
🪛 LanguageTool
[locale-violation] ~146-~146: In American English, “take a look” is more commonly used.
Context: ...face` method to bundle these two steps. Have a look at [a real-life example in the x/auth m...(HAVE_A_LOOK)
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
📒 Files selected for processing (3)
- docs/learn/advanced/05-encoding.md (6 hunks)
- docs/learn/advanced/08-events.md (3 hunks)
- docs/learn/advanced/11-runtx_middleware.md (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/learn/advanced/08-events.md
🧰 Additional context used
📓 Path-based instructions (2)
docs/learn/advanced/05-encoding.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"docs/learn/advanced/11-runtx_middleware.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
🪛 LanguageTool
docs/learn/advanced/05-encoding.md
[locale-violation] ~146-~146: In American English, “take a look” is more commonly used.
Context: ...face` method to bundle these two steps. Have a look at [a real-life example in the x/auth m...(HAVE_A_LOOK)
docs/learn/advanced/11-runtx_middleware.md
[uncategorized] ~50-~50: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...y default, that panic would be recovered and an error message will be printed to log...(COMMA_COMPOUND_SENTENCE_2)
[typographical] ~50-~50: It seems that a comma is missing.
Context: ...ill be printed to log. To override that behaviour we should register a custom RecoveryHan...(IN_ORDER_TO_VB_COMMA)
🔇 Additional comments (4)
docs/learn/advanced/11-runtx_middleware.md (4)
11-11: LGTM: Version reference correctly updatedThe version update to v0.52.0-beta.2 is consistent with the documentation updates across the SDK.
16-16: LGTM: Interface reference link updatedThe version in the code reference link is correctly updated to match the SDK version.
36-36: LGTM: Correct use of contractionThe grammatical correction from "Lets" to "Let's" is appropriate.
28-30: LGTM: Code reference link updated correctlyThe version update in the code reference link maintains consistency with the SDK version.
✅ Verification successful
Code reference link and line numbers are accurate
The referenced code at lines 549-554 in baseapp/baseapp.go correctly points to the
AddRunTxRecoveryHandlerfunction implementation in the v0.52.0-beta.2 version of cosmos-sdk.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the referenced line numbers point to the correct code gh api repos/cosmos/cosmos-sdk/contents/baseapp/baseapp.go?ref=v0.52.0-beta.2 | jq -r '.content' | base64 -d | sed -n '549,554p'Length of output: 202
Script:
#!/bin/bash # Try alternative approach to fetch and verify the content curl -s -L "https://raw.githubusercontent.com/cosmos/cosmos-sdk/v0.52.0-beta.2/baseapp/baseapp.go" | sed -n '549,554p'Length of output: 384
(cherry picked from commit 9f79dd3)
Co-authored-by: Julián Toledano <[email protected]>
Description
Ref:
#21429
Updatea:
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
BaseApp,Encoding,gRPC REST,CLI,Events,Telemetry, andRunTx Middlewaredocuments.