-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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: fix dead link in UPGRADING.md
#23059
Conversation
📝 WalkthroughWalkthroughThe pull request focuses on updating the Changes
Sequence DiagramsequenceDiagram
participant Dev as Developer
participant App as SimApp
participant Module as Modules
participant Context as Context Management
Dev->>App: Initialize Application
App->>Context: Transition to new Context handling
App->>Module: Update Module Interfaces
Module->>App: Implement New Method Signatures
Dev->>App: Migrate Upgrade Handlers
App->>Module: Apply Consensus Parameters
Possibly related PRs
Suggested labels
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
|
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: 0
🧹 Nitpick comments (4)
simapp/CHANGELOG.md (4)
24-30
: Consider clarifying the relationship with UPGRADING.mdWhile the introduction is clear, it would be helpful to explicitly state that this changelog complements the SimApp section in UPGRADING.md by providing detailed implementation changes.
This changelog is aimed to help developers understand the wiring changes between SDK versions. -It is an exhaustive list of changes that completes the SimApp section in the [UPGRADING.md](https://github.com/cosmos/cosmos-sdk/blob/main/UPGRADING.md#simapp) +It provides an exhaustive list of implementation-specific changes that complements the high-level migration guide in [UPGRADING.md](https://github.com/cosmos/cosmos-sdk/blob/main/UPGRADING.md#simapp).
31-50
: Restructure changes according to the guidelinesThe changelog entries should be grouped by type (Features, Improvements, API Breaking, etc.) as specified in the guidelines.
Consider reorganizing the changes like this:
## v0.50 to v0.52 Always refer to the [UPGRADING.md](https://github.com/cosmos/cosmos-sdk/blob/main/UPGRADING.md) to understand the changes. +### Features +* Wire new SDK modules (`epochs`, `accounts`, `protocolpool`). +* Add example of how to define a custom mint function in `simapp/mint_fn.go`. +* Add address codec in client context and signing context. + +### Improvements +* Update `export` function to make use of the new module collections API. +* Update testnet command to match new module APIs. +* [#20740] Update `genutilcli.Commands` to use the genutil modules from the module manager. +* [#20490] Refactor simulations to make use of `testutil/sims` instead of `runsims`. + +### API Breaking +* Update module path to new vanity url. +* Remove the crisis module from simapp. +* [#20485] The signature of `x/upgrade/types.UpgradeHandler` has changed...
51-51
: Clarify or remove the TODO commentThe TODO comment "move changelog.md elements to here" lacks context and specificity about what needs to be moved.
Would you like me to help clarify this TODO or open an issue to track this task?
53-63
: Consider adding version release datesFor better historical tracking, consider adding release dates to each version section, following the principle mentioned in the guidelines: "The release date of each version is displayed."
-## v0.47 to v0.50 +## v0.47 to v0.50 (Released 2023-XX-XX)
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
simapp/CHANGELOG.md
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
simapp/CHANGELOG.md (1)
Pattern **/*.md
: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
🔇 Additional comments (1)
simapp/CHANGELOG.md (1)
1-22
: LGTM! Well-structured changelog guidelines.
The header section follows the Keep a Changelog standard and provides clear guidance for contributors.
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.
Hey! The solution to this is actually to link the simapp/CHANGELOG.md from the release/v0.52.branch in the upgrading.md, not to re-add it in main
ba23692
to
7cb8574
Compare
Hi sir, I have updated the link to https://github.com/cosmos/cosmos-sdk/blob/release/v0.52.x/simapp/CHANGELOG.md |
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
Plan: Pro
📒 Files selected for processing (1)
UPGRADING.md
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
UPGRADING.md (1)
Pattern **/*.md
: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
@@ -62,7 +62,7 @@ It is additional to the changes described here. | |||
|
|||
In this section we describe the changes made in Cosmos SDK' SimApp. | |||
**These changes are directly applicable to your application wiring.** | |||
Please read this section first, but for an exhaustive list of changes, refer to the [CHANGELOG](https://github.com/cosmos/cosmos-sdk/blob/main/simapp/CHANGELOG.md). | |||
Please read this section first, but for an exhaustive list of changes, refer to the [CHANGELOG](https://github.com/cosmos/cosmos-sdk/blob/release/v0.52.x/simapp/CHANGELOG.md). |
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.
Fix dead link to SimApp CHANGELOG
The link to simapp/CHANGELOG.md
is broken as the file was removed in PR #23009. Since this section discusses important changes in SimApp that are directly applicable to application wiring, consider either:
- Restoring the CHANGELOG file with the relevant information, or
- Including the important changes directly in this section of the upgrade guide.
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.
Thanks!
Co-authored-by: Hieu Vu <[email protected]> (cherry picked from commit 2596c7f) # Conflicts: # UPGRADING.md
Description
simapp/CHANGELOG.md
was removed in PR #23009 , but this file is required inUPGRADING.md
.Maybe this file was mis-deleted and should be recovered?
Summary by CodeRabbit
UPGRADING.md
to reflect significant changes in the Cosmos SDK.x/params
module and added thex/protocolpool
module.