docs: reword upgrading.md #19900
docs: reword upgrading.md #19900
Conversation
WalkthroughWalkthroughThis update involves the relocation of Changes
Possibly related issues
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 4
Configuration used: .coderabbit.yml
Files selected for processing (2)
- UPGRADING.md (2 hunks)
- scripts/go-mod-tidy-all.sh (1 hunks)
Path instructions used (1)
UPGRADING.md (1)
**/*.md
Assess the documentation for misspellings, grammatical errors, missing documentation and correctness
Additional comments (24)
scripts/go-mod-tidy-all.sh (2)
- 17-17: The addition of a check for
nixbefore runninggomod2nixis a good practice. It ensures that the necessary tools are available, enhancing the script's reliability.- 13-20: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [17-25]
When installing
gomod2nixif it's not found, consider verifying thatgois available and properly configured. This can prevent potential issues for users who might not havegoset up correctly. A simple check before attempting the installation could be beneficial:if ! command -v go &> /dev/null; then echo "Go is not installed or not in PATH, which is required for installing gomod2nix." exit 1 fiUPGRADING.md (22)
- 148-155: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [4-4]
The term "SimApp" might be unfamiliar to some readers. Consider adding a brief explanation or a link to more information about SimApp for clarity.
- 148-155: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [15-15]
The sentence structure here is a bit unclear. Consider rephrasing for better readability.
Suggestion: "This refactoring is part of the effort to abstract the SDK from the global bech32 config."
- 148-155: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [19-19]
It's generally a good practice to avoid using terms like "usually" without further explanation. If
root.gois the common location for the application client, consider stating it more definitively.
- 148-155: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [30-30]
The term "depinject" might not be clear to all readers. Consider adding a brief explanation or a link to more information about dependency injection in the Cosmos SDK.
- 148-155: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [48-48]
The phrase "For depinject users" could be more inclusive by mentioning "For users utilizing dependency injection," making it clearer to those unfamiliar with the term "depinject."
- 148-155: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [52-52]
The term "non depinject users" could be confusing. Consider rephrasing to "For users not utilizing dependency injection" for clarity.
- 148-155: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [62-62]
The phrase "as explained in the genesis interface update" lacks a direct link or further explanation. Ensure that readers can easily find the referenced section.
- 148-155: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [72-72]
The term "nonces" might not be familiar to all readers. Consider adding a brief explanation or a link to more information about nonces in the context of transactions.
- 148-155: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [94-94]
The phrase "as early as possible" is somewhat vague. Consider specifying where in the AnteHandler chain this decorator should ideally be placed.
- 148-155: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [108-108]
The term "SnapshotManager" might not be clear to all readers. Consider adding a brief explanation or a link to more information about the SnapshotManager and its role.
- 148-155: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [120-120]
The phrase "Note, this is critical" could be emphasized for importance. Consider using a stronger statement like "It is crucial to note that closing the unordered tx manager is essential for ensuring..."
- 148-155: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [146-146]
The transition from hosting Tendermint protos to using
buf.buildfor CometBFT might not be clear to all readers. Consider adding a brief explanation of the significance of this change.
- 151-151: The phrase "This change is optional" could be misleading. Clarify under what circumstances a module developer might choose not to make this change.
- 148-155: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [159-159]
The change in
MsgSimulatorFncould impact existing simulations. Consider advising readers to review their simulation setups for compatibility.
- 148-155: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [169-169]
The introduction of the Core API is a significant change. Recommend providing examples or links to documentation for modules to transition smoothly.
- 148-155: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [188-188]
The change in
HasRegisterInterfacescould impact module interoperability. Suggest providing a template or example implementation for clarity.
- 148-155: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [196-196]
The move of dependency injection functions could lead to import errors. Recommend providing a migration guide or script to update imports automatically.
- 148-155: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [202-202]
The requirement to migrate to v0.50 before upgrading to v0.51 could be emphasized more strongly to prevent migration issues.
- 148-155: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [206-206]
The changes to genesis interfaces are critical for module developers. Suggest providing detailed examples of how to implement these changes.
- 148-155: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [223-223]
The migration to collections is a significant change. Recommend providing a detailed guide or examples for modules to transition smoothly.
- 445-445: The link to annotations documentation is crucial for developers. Ensure that the link is accessible and up-to-date.
- 148-155: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [449-449]
The removal of
gogoproto.goproto_stringer = falsecould impact logging and debugging. Advise reviewing the usage ofString()methods in critical paths.
scripts/go-mod-tidy-all.sh
Outdated
| # automatically. | ||
| cd simapp | ||
| if command -v nix &> /dev/null | ||
| nix develop .. -c gomod2nix | ||
| then | ||
| nix develop .. -c gomod2nix | ||
| if ! command -v gomod2nix &> /dev/null | ||
| then | ||
| echo "gomod2nix could not be found in PATH, installing..." |
There was a problem hiding this comment.
📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [5-5]
Consider using a while read loop or find -exec for more robust handling of filenames, especially those containing spaces or newlines. For example:
find . -name go.mod -exec sh -c 'echo "Updating $1"; DIR=$(dirname "$1"); (cd "$DIR"; go mod tidy)' _ {} \;📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [7-7]
Variables should be double-quoted to prevent globbing and word splitting. This is particularly important for paths that might contain spaces. For example:
DIR=$(dirname "$modfile")📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [8-8]
To ensure the script handles directory paths correctly, especially those with spaces, wrap $DIR in double quotes:
(cd "$DIR"; go mod tidy)
There was a problem hiding this comment.
Actionable comments posted: 0
Review Status
Configuration used: .coderabbit.yml
Files selected for processing (1)
- UPGRADING.md (2 hunks)
Additional Context Used
Path-based Instructions (1)
UPGRADING.md (1)
**/*.md:
Assess the documentation for misspellings, grammatical errors, missing documentation and correctness
Additional comments not posted (2)
UPGRADING.md (2)
151-151: The relocation ofcodectypes.Anyto a new package is correctly documented. However, it's important to ensure that all references tocodectypes.Anythroughout the project are updated to reflect this change.
445-445: The link to the annotations documentation is a valuable resource for developers. It's crucial to verify that all module developers are aware of this change and update their protobuf files accordingly.
* main: fix(simapp): use correct store keys for upgrade (cosmos#19987) build(ci): Remove staticmajor due to false positives (cosmos#19989) build(deps): Bump google.golang.org/grpc from 1.63.0 to 1.63.2 (cosmos#19982) chore: linting fixes (cosmos#19978) docs: reword upgrading.md (cosmos#19900)
Description
ref: #19869 (comment)
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.
I have...
Summary by CodeRabbit
codectypes.Anyand deprecating the Amino JSON codec.