-
Notifications
You must be signed in to change notification settings - Fork 4.1k
docs: fix grammar and spelling errors in building apps/modules guides #25136
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
Conversation
📝 WalkthroughWalkthroughDocumentation updates across ABCI, app-building, and module-building guides: grammar/wording fixes, minor formatting, a new normative note that VerifyVoteExtensions must be deterministic, and an added example showing a testnet CLI hook with a helper constructor function. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes ✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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 comments (1)
docs/docs/build/building-apps/05-app-testnet.md (1)
198-200: Update AddTestnetCreatorCommand invocation and document its purposeWe’ve confirmed that
AddTestnetCreatorCommandis defined as:func AddTestnetCreatorCommand( rootCmd *cobra.Command, appCreator types.AppCreator, addStartFlags types.ModuleInitFlags, )—there is no
defaultNodeHomeparameter. Please:• In
docs/docs/build/building-apps/05-app-testnet.md(around lines 198–200), change:- server.AddTestnetCreatorCommand(rootCmd, simapp.DefaultNodeHome, newTestnetApp, addModuleInitFlags) + server.AddTestnetCreatorCommand(rootCmd, newTestnetApp, addModuleInitFlags)• Add a brief description for discoverability, e.g. before or after the diff block:
“The
AddTestnetCreatorCommandhook registers atestnetsubcommand that scaffolds and exports a local testnet using the node’s existing state directory.”
🧹 Nitpick comments (7)
docs/docs/build/building-modules/03-msg-services.md (1)
77-79: Fix event literal; also replace leading tab to satisfy lint (MD010).The corrected struct literal is right. Replace the hard tab with spaces.
- ctx.EventManager().EmitTypedEvent( - &group.EventABC{Key1: Value1, Key2: Value2}) + ctx.EventManager().EmitTypedEvent( + &group.EventABC{Key1: Value1, Key2: Value2})docs/docs/build/abci/00-introduction.md (2)
22-22: Minor style nit: simplify “in conjunction with”.Optional: shorten for concision.
-By default, the Cosmos SDK provides `PrepareProposalHandler`, used in conjunction with an application specific mempool. +By default, the Cosmos SDK provides `PrepareProposalHandler`, used together with an application‑specific mempool.
42-47: Good addition: determinism requirement is explicit. Plus optional brevity tweak.Clear normative “MUST be deterministic” is helpful. Also consider trimming “in order to”.
-`VerifyVoteExtensions` is performed on every validator multiple times in order to verify other validators' vote extensions. +`VerifyVoteExtensions` is performed on every validator multiple times to verify other validators' vote extensions.docs/docs/build/building-apps/02-app-mempool.md (2)
51-53: Concision nit: trim “in order to”.Optional edit for brevity.
-The nonce mempool is a mempool that keeps transactions from a sender sorted by nonce in order to avoid the issues with nonces. +The nonce mempool keeps transactions from a sender sorted by nonce to avoid nonce-related issues.
90-93: Callback phrasing improved; add tiny grammar fix.Good clarification on callbacks. Suggest adding “a” and “is” for grammar.
-The priority nonce mempool provides mempool options allowing the application to set callback(s). +The priority nonce mempool provides options allowing the application to set callback(s). @@ -* **TxReplacement**: Sets a callback to be called when duplicate transaction nonce detected during mempool insert. Application can define a transaction replacement rule based on tx priority or certain transaction fields. +* **TxReplacement**: Sets a callback to be called when a duplicate transaction nonce is detected during mempool insert. The application can define a transaction replacement rule based on tx priority or certain transaction fields.docs/docs/build/building-apps/05-app-testnet.md (1)
205-235: Refine panics, fix grammar, and please manually verify symbol/key existenceNit fixes and clearer panic messaging—please update your snippet as follows and confirm that
servertypes.Application,simapp.InitSimAppForTestnetand all AppOptions keys/types actually exist in your codebase (we couldn’t locate them via local search).- // Create an app and type cast to an SimApp + // Create an app and type cast to a SimApp app := newApp(logger, db, traceStore, appOpts) simApp, ok := app.(*simapp.SimApp) if !ok { - panic("app created from newApp is not of type simApp") + panic(fmt.Sprintf("app created from newApp is not a SimApp, got %T", app)) } newValAddr, ok := appOpts.Get(server.KeyNewValAddr).(bytes.HexBytes) if !ok { - panic("newValAddr is not of type bytes.HexBytes") + panic("newValAddr (server.KeyNewValAddr) is not bytes.HexBytes") } newValPubKey, ok := appOpts.Get(server.KeyUserPubKey).(crypto.PubKey) if !ok { - panic("newValPubKey is not of type crypto.PubKey") + panic("newValPubKey (server.KeyUserPubKey) is not crypto.PubKey") } newOperatorAddress, ok := appOpts.Get(server.KeyNewOpAddr).(string) if !ok { - panic("newOperatorAddress is not of type string") + panic("newOperatorAddress (server.KeyNewOpAddr) is not string") } upgradeToTrigger, ok := appOpts.Get(server.KeyTriggerTestnetUpgrade).(string) if !ok { - panic("upgradeToTrigger is not of type string") + panic("upgradeToTrigger (server.KeyTriggerTestnetUpgrade) is not string") } return simapp.InitSimAppForTestnet(simApp, newValAddr, newValPubKey, newOperatorAddress, upgradeToTrigger)• Ensure that:
servertypes.Applicationis the declared return type ofnewTestnetApp.simapp.InitSimAppForTestnetsignature matches your imports and types.- Each
server.Key*constant and its expected Go type (bytes.HexBytes,crypto.PubKey,string) exists in your code.docs/docs/build/building-apps/01-app-go-di.md (1)
9-9: Style tweak for flowConsider “makes wiring
app.gomuch easier” (drop “of” and the article) for slightly smoother reading.-The Cosmos SDK makes wiring of an `app.go` much easier thanks to [runtime](./00-runtime.md) and app wiring. +The Cosmos SDK makes wiring `app.go` much easier thanks to [runtime](./00-runtime.md) and app wiring.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Knowledge Base: Disabled due to Reviews > Disable Knowledge Base setting
📒 Files selected for processing (10)
docs/docs/build/abci/00-introduction.md(2 hunks)docs/docs/build/abci/02-process-proposal.md(1 hunks)docs/docs/build/building-apps/01-app-go-di.md(3 hunks)docs/docs/build/building-apps/02-app-mempool.md(3 hunks)docs/docs/build/building-apps/03-app-upgrade.md(2 hunks)docs/docs/build/building-apps/04-vote-extensions.md(1 hunks)docs/docs/build/building-apps/05-app-testnet.md(1 hunks)docs/docs/build/building-modules/01-module-manager.md(4 hunks)docs/docs/build/building-modules/02-messages-and-queries.md(2 hunks)docs/docs/build/building-modules/03-msg-services.md(2 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
docs/docs/build/building-apps/01-app-go-di.md
70-70: Hard tabs
Column: 1
(MD010, no-hard-tabs)
docs/docs/build/building-modules/03-msg-services.md
78-78: Hard tabs
Column: 1
(MD010, no-hard-tabs)
🪛 LanguageTool
docs/docs/build/building-apps/02-app-mempool.md
[style] ~51-~51: Consider a more concise word here.
Context: ...nsactions from a sender sorted by nonce in order to avoid the issues with nonces. It works...
(IN_ORDER_TO_PREMIUM)
docs/docs/build/abci/00-introduction.md
[style] ~22-~22: ‘in conjunction with’ might be wordy. Consider a shorter alternative.
Context: ...provides PrepareProposalHandler, used in conjunction with an application specific mempool. A cust...
(EN_WORDINESS_PREMIUM_IN_CONJUNCTION_WITH)
[style] ~42-~42: Consider a more concise word here.
Context: ...ormed on every validator multiple times in order to verify other validators' vote extension...
(IN_ORDER_TO_PREMIUM)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: main
- GitHub Check: main
- GitHub Check: Summary
🔇 Additional comments (15)
docs/docs/build/abci/02-process-proposal.md (1)
10-12: Grammar fix reads better; no behavior change.The added “that” improves flow without altering meaning.
docs/docs/build/building-apps/04-vote-extensions.md (1)
77-77: Formatting tweak improves consistency.Left-aligning the intro sentence before the code block is good.
docs/docs/build/building-modules/02-messages-and-queries.md (2)
50-56: Subject–verb agreement fix is correct.“signers … are automated” aligns with the plural subject.
131-137: Wording is clearer and more precise.“access store keys directly” reads better; no semantic change.
docs/docs/build/building-modules/03-msg-services.md (1)
36-40: Clarification about retrieving sdk.Context is accurate.“from the context.Context parameter” is the right phrasing.
docs/docs/build/building-modules/01-module-manager.md (4)
32-48: Pluralization and HasServices reference corrected.“that define specific functionalities” and pointing to HasServices are both accurate.
145-151: Invariants description clarified.“allows checking” improves readability.
157-171: Services registration language fixed.Clearer description; consistent with appmodule.HasService/module.HasServices.
199-200: Minor grammar and completeness improvement.EndBlocker description reads correctly and mentions HasABCIEndBlock context.
docs/docs/build/building-apps/03-app-upgrade.md (2)
25-33: Punctuation and hyphenation fixes improve clarity.“e.g.”, “app‑specific rather than module‑specific,” and sentence flow are all improvements.
59-68: “set up” and phrasing around PreBlocker are correct.Clearer instructions for wiring the upgrade keeper and pre-blocker.
docs/docs/build/building-apps/02-app-mempool.md (2)
63-63: Minor wording consistency.Article/backticks around
maxTxare consistent with style elsewhere.
86-86: SamemaxTxclarification here looks good.Keeps docs consistent across mempool variants.
docs/docs/build/building-apps/05-app-testnet.md (1)
28-28: Grammar fix LGTMReads clearly: “to migrate” is the correct infinitive usage.
docs/docs/build/building-apps/01-app-go-di.md (1)
43-44: Clarity improvement LGTM“the order in which …” is the correct phrasing; reads clearly.
| :::tip | ||
| The example above shows how to create an `AppConfig` using Go. However, it is also possible to create an `AppConfig` using YAML, or JSON. | ||
| The configuration can then be embed with `go:embed` and read with [`appconfig.LoadYAML`](https://pkg.go.dev/cosmossdk.io/core/appconfig#LoadYAML), or [`appconfig.LoadJSON`](https://pkg.go.dev/cosmossdk.io/core/appconfig#LoadJSON), in `app_di.go`. | ||
| The configuration can then be embedded with `go:embed` and read with [`appconfig.LoadYAML`](https://pkg.go.dev/cosmossdk.io/core/appconfig#LoadYAML), or [`appconfig.LoadJSON`](https://pkg.go.dev/cosmossdk.io/core/appconfig#LoadJSON), in `app_di.go`. |
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.
💡 Verification agent
🧩 Analysis chain
Fix markdownlint MD010 (hard tabs) and wording
- Lint: Replace the leading hard tab(s) with spaces to satisfy MD010.
- Wording “embedded” is correct — keep as is.
- The configuration can then be embedded with `go:embed` and read with [`appconfig.LoadYAML`](https://pkg.go.dev/cosmossdk.io/core/appconfig#LoadYAML), or [`appconfig.LoadJSON`](https://pkg.go.dev/cosmossdk.io/core/appconfig#LoadJSON), in `app_di.go`.
+ The configuration can then be embedded with `go:embed` and read with [`appconfig.LoadYAML`](https://pkg.go.dev/cosmossdk.io/core/appconfig#LoadYAML), or [`appconfig.LoadJSON`](https://pkg.go.dev/cosmossdk.io/core/appconfig#LoadJSON), in `app_di.go`.Quick check for tabs on this file:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Lines with tabs (should be empty after fix):"
rg -nP "\t" docs/docs/build/building-apps/01-app-go-di.md || trueLength of output: 194
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Context around tabs:"
rg -nP '\t' -n -C2 docs/docs/build/building-apps/01-app-go-di.mdLength of output: 301
Fix remaining hard tab (MD010)
Another hard tab was found at line 145 in the code block. Please replace it with spaces to satisfy MD010. The wording (“embedded”) remains correct.
• File: docs/docs/build/building-apps/01-app-go-di.md
Line 145: replace leading tab on panic(err) with two spaces
@@ docs/docs/build/building-apps/01-app-go-di.md
- panic(err)
+ panic(err)Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
70-70: Hard tabs
Column: 1
(MD010, no-hard-tabs)
🤖 Prompt for AI Agents
In docs/docs/build/building-apps/01-app-go-di.md (review context around line 70,
actual offending line at 145): there is a hard tab at the start of the code line
containing `panic(err)`; replace that leading tab with two spaces so the code
block uses spaces instead of a hard tab and re-check for any other tabs in the
block.
Description
Summary by CodeRabbit