-
Notifications
You must be signed in to change notification settings - Fork 301
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
chore: move pending-release to release-notes #3988
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThe changes involve updating the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
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
🧹 Outside diff range and nitpick comments (2)
docs/release-notes/release-notes.md (2)
24-34
: LGTM: Valuable information added for v3.0.0-rc0 Node Operators.The new section provides crucial information for node operators upgrading to v3.0.0-rc0, including:
- The requirement to enable the BBR congestion control algorithm.
- Instructions for configuring the node's mempool
ttl-num-blocks
.These updates are essential for ensuring proper node operation after the upgrade.
Consider enhancing the bash command for better user experience:
- sed -i 's/ttl-num-blocks = 5/ttl-num-blocks = 12/' ~/.celestia-app/config/config.toml + sed -i 's/^ttl-num-blocks =.*/ttl-num-blocks = 12/' ~/.celestia-app/config/config.tomlThis change makes the command more robust, as it will work regardless of the current
ttl-num-blocks
value.
35-37
: LGTM: Important update for v3.0.0-rc0 Library Consumers.The new section provides crucial information about the relocation of namespace and share constants to the
go-square
repository. This is an important change that library consumers need to be aware of.Consider enhancing this section with more detailed guidance:
- Provide the new import path for these constants.
- Include a brief code example showing how to use the relocated constants.
- Mention any potential breaking changes this relocation might cause and how to address them.
This additional information would help library consumers adapt to the changes more easily.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
- docs/release-notes/pending-release.md (0 hunks)
- docs/release-notes/release-notes.md (1 hunks)
💤 Files with no reviewable changes (1)
- docs/release-notes/pending-release.md
🧰 Additional context used
🪛 LanguageTool
docs/release-notes/release-notes.md
[style] ~22-~22: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...f8d1c60e5511104deed8bc361/inclusion). - celestia-app v1.x had a lot of functionality include...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~22-~22: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...f functionality included in the signer. celestia-app v2.x splits a txClient from the signer....(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🔇 Additional comments (2)
docs/release-notes/release-notes.md (2)
7-14
: LGTM: Improved clarity and added important information for node operators.The changes in this section enhance the release notes by:
- Specifying the version number in the heading, which improves clarity.
- Adding crucial information about enabling the BBR congestion control algorithm, which is important for node operators.
These updates provide valuable guidance for node operators upgrading to v2.0.0.
15-23
: LGTM: Improved heading clarity.The update to the heading enhances clarity by specifying the version number (v2.0.0) for Library Consumers.
Note: The static analysis tool flagged repeated sentence beginnings, but in technical documentation, this repetition can actually aid clarity by maintaining a consistent structure. In this case, it's appropriate to keep the current wording.
🧰 Tools
🪛 LanguageTool
[style] ~22-~22: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...f8d1c60e5511104deed8bc361/inclusion). - celestia-app v1.x had a lot of functionality include...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~22-~22: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...f functionality included in the signer. celestia-app v2.x splits a txClient from the signer....(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
docs/release-notes/release-notes.md
Outdated
|
||
## [v3.0.0-rc0](https://github.com/celestiaorg/celestia-app/releases/tag/v3.0.0-rc0) | ||
|
||
### Node Operators (v3.0.0-rc0) |
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.
I'd be fine just saying v3.0.0, or all v3.*+
tags or something so we don't have to update this constantly. ie we're going to cut the arabica tag soon
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.
Agreed, will change
ad34bf6
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
🧹 Outside diff range and nitpick comments (2)
docs/release-notes/release-notes.md (2)
26-33
: LGTM with a minor suggestion: Node Operators section for v3.0.0The content provides clear and helpful instructions for node operators, which aligns well with the PR objectives. The inclusion of specific commands is particularly useful.
For consistency with the v2.0.0 section, consider updating the heading format:
### Node Operators (v3.0.0)
35-37
: LGTM with a minor suggestion: Library Consumers section for v3.0.0The content provides clear information about changes relevant to library consumers, which aligns well with the PR objectives.
For consistency with the v2.0.0 section, consider updating the heading format:
### Library Consumers (v3.0.0)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- docs/release-notes/release-notes.md (1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/release-notes/release-notes.md
[style] ~22-~22: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...f8d1c60e5511104deed8bc361/inclusion). - celestia-app v1.x had a lot of functionality include...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~22-~22: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...f functionality included in the signer. celestia-app v2.x splits a txClient from the signer....(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🔇 Additional comments (3)
docs/release-notes/release-notes.md (3)
7-7
: LGTM: Version numbers added to headingsThe addition of version numbers to the "Node Operators" and "Library Consumers" headings improves clarity and aligns with the previous review suggestions.
Also applies to: 15-15
24-24
: LGTM: New section for v3.0.0 addedThe addition of the v3.0.0 section heading is appropriate and consistent with the document structure.
7-37
: Overall assessment: Effective update to release notesThe changes to this file successfully incorporate the pending release information for v3.0.0 into the main release notes document. The additions provide clear and helpful information for both node operators and library consumers, aligning well with the PR objectives.
The structure is consistent with the existing format, and the content is informative and well-organized. The minor formatting suggestions for headings, if implemented, will further improve consistency across versions.
Great job on updating the release notes to provide comprehensive guidance for users upgrading to new versions of celestia-app.
🧰 Tools
🪛 LanguageTool
[style] ~22-~22: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...f8d1c60e5511104deed8bc361/inclusion). - celestia-app v1.x had a lot of functionality include...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~22-~22: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...f functionality included in the signer. celestia-app v2.x splits a txClient from the signer....(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
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.
I would prefer if the latest release was always put at the top.
And we should not remove the pending-release
document but rather update it to v4
```bash | ||
sed -i 's/ttl-num-blocks = 5/ttl-num-blocks = 12/' ~/.celestia-app/config/config.toml | ||
``` |
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.
Does this need to be indented?
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.
Also adds a line for node operators to modify their mempool ttl-num-blocks config (cherry picked from commit a2f10ed)
Also adds a line for node operators to modify their mempool ttl-num-blocks config