-
Notifications
You must be signed in to change notification settings - Fork 293
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: release notes #3794
docs: release notes #3794
Conversation
WalkthroughWalkthroughThe changes introduce a new Changes
Assessment against linked issues
Recent review detailsConfiguration used: .coderabbit.yaml Files selected for processing (2)
Additional context usedLanguageTool
Additional comments not posted (4)
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 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, codebase verification and nitpick comments (1)
RELEASES.md (1)
13-20
: Improve readability for library consumers.The list of changes for library consumers can be improved for readability by varying sentence structure and avoiding repetition.
- - celestia-app v1.x had a shares package. celestia-app v2.x uses [go-square/shares](https://github.com/celestiaorg/go-square/tree/c8242f96a844956f8d1c60e5511104deed8bc361/shares) - - celestia-app v1.x had a blob.types package with `CreateCommitment`. celestia-app v2.x uses `CreateCommitment` from the [go-square/inclusion](https://github.com/celestiaorg/go-square/tree/c8242f96a844956f8d1c60e5511104deed8bc361/inclusion). - - celestia-app v1.x had a lot of functionality included in the signer. celestia-app v2.x split a txClient from the signer. + - The shares package from celestia-app v1.x has been moved to [go-square/shares](https://github.com/celestiaorg/go-square/tree/c8242f96a844956f8d1c60e5511104deed8bc361/shares) in v2.x. + - The `CreateCommitment` function, originally in the blob.types package, is now available in [go-square/inclusion](https://github.com/celestiaorg/go-square/tree/c8242f96a844956f8d1c60e5511104deed8bc361/inclusion) for v2.x. + - Functionality previously included in the signer has been separated, with the introduction of a txClient in v2.x.Tools
LanguageTool
[style] ~20-~20: 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] ~20-~20: 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 split 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.
LGTM!, left some comments.
Co-authored-by: Sanaz Taheri <35961250+staheri14@users.noreply.github.com>
Co-authored-by: Sanaz Taheri <35961250+staheri14@users.noreply.github.com>
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 can't speak to whether every single detail required for the upgrade is included, but from my perspective as a general reader, it looks good to me!
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.
Not if this is your plan, but I think the latest release should always be at the top. Also I'm not sure if we should keep the pending release information in the same document. I don't want to confuse users about releases that aren't ready
RELEASES.md
Outdated
### Node operators | ||
|
||
- Consensus node operators should enable the BBR (Bottleneck Bandwidth and Round-trip propagation time) congestion control algorithm. See [#3774](https://github.com/celestiaorg/celestia-app/pull/3774). |
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.
Don't we want this mentioned in v2?
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 think it's optional in v2 and required in v3 so I chose to document it in v3. We don't have the warning yet in v2 so I don't think we need to document in in the v2 section.
When we add the warning to v2, we can also copy a variant of this that says "optional" to the v2 section.
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.
cc: @evan-forbes for thoughts
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 inform users in v2. We want all of them to upgrade in v2
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.
Can do. Then this PR is blocked on a variant of #3774 that includes instructions on how to enable BBR on the v2.x branch
Co-authored-by: Callum Waters <cmwaters19@gmail.com>
I'll create a new document like |
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, codebase verification and nitpick comments (6)
RELEASES.md (6)
18-18
: Consider rephrasing for clarity.The existing comment suggests rephrasing the sentence for clarity. Here's a refined suggestion:
- celestia-app v1.x had a shares package. celestia-app v2.x uses [go-square/shares](https://github.com/celestiaorg/go-square/tree/c8242f96a844956f8d1c60e5511104deed8bc361/shares) + In celestia-app v1.x, the shares package was included. In celestia-app v2.x, it is replaced by [go-square/shares](https://github.com/celestiaorg/go-square/tree/c8242f96a844956f8d1c60e5511104deed8bc361/shares).
19-19
: Add missing article for grammatical correctness.The article "the" is missing before
CreateCommitment
function.- celestia-app v1.x had a blob.types package with `CreateCommitment` function. celestia-app v2.x uses `CreateCommitment` function from the [go-square/inclusion](https://github.com/celestiaorg/go-square/tree/c8242f96a844956f8d1c60e5511104deed8bc361/inclusion). + celestia-app v1.x had a blob.types package with the `CreateCommitment` function. celestia-app v2.x uses the `CreateCommitment` function from the [go-square/inclusion](https://github.com/celestiaorg/go-square/tree/c8242f96a844956f8d1c60e5511104deed8bc361/inclusion).Tools
LanguageTool
[uncategorized] ~19-~19: You might be missing the article “the” here.
Context: ...tmentfunction. celestia-app v2.x uses
CreateCommitment` function from the [go-...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
20-20
: Rephrase to avoid repetition.The sentence structure is repetitive. Consider rephrasing for better readability.
- celestia-app v1.x had a lot of functionality included in the signer. celestia-app v2.x splits a txClient from the signer. See [#3433](https://github.com/celestiaorg/celestia-app/pull/3433). + In celestia-app v1.x, much of the functionality was included in the signer. In celestia-app v2.x, a txClient is separated from the signer. See [#3433](https://github.com/celestiaorg/celestia-app/pull/3433).Tools
LanguageTool
[style] ~20-~20: 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] ~20-~20: 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)
22-22
: Clarify the release status.The existing comment suggests indicating the release status. Consider adding a note about the draft status:
## v3.0.0 [Unreleased] + _Note: This section is a draft and subject to change._
26-26
: Consider adding context for BBR.The existing comment suggests mentioning BBR in version 2. Consider providing more context or a link to documentation:
- Consensus node operators should enable the BBR (Bottleneck Bandwidth and Round-trip propagation time) congestion control algorithm. See [#3774](https://github.com/celestiaorg/celestia-app/pull/3774). + Consensus node operators should enable the BBR (Bottleneck Bandwidth and Round-trip propagation time) congestion control algorithm, which optimizes network performance. For more details, see [#3774](https://github.com/celestiaorg/celestia-app/pull/3774).
30-30
: Provide additional context or examples.Consider adding examples or more context to help library consumers understand the impact of the changes.
- Namespace and share constants in the `appconsts` package were moved to [celestiaorg/go-square](https://github.com/celestiaorg/go-square). See [#3765](https://github.com/celestiaorg/celestia-app/pull/3765). + Namespace and share constants in the `appconsts` package were moved to [celestiaorg/go-square](https://github.com/celestiaorg/go-square). This change may affect how you reference these constants in your code. See [#3765](https://github.com/celestiaorg/celestia-app/pull/3765) for more details.
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, codebase verification and nitpick comments (1)
RELEASES.md (1)
17-20
: Improve sentence variety for readability.The section contains repetitive sentence beginnings. Consider rephrasing to enhance readability.
Use this diff to improve the sentence structure:
- celestia-app v1.x had a shares package. celestia-app v2.x uses [go-square/shares](https://github.com/celestiaorg/go-square/tree/c8242f96a844956f8d1c60e5511104deed8bc361/shares) - celestia-app v1.x had a blob.types package with `CreateCommitment` function. celestia-app v2.x uses `CreateCommitment` function from the [go-square/inclusion](https://github.com/celestiaorg/go-square/tree/c8242f96a844956f8d1c60e5511104deed8bc361/inclusion). - celestia-app v1.x had a lot of functionality included in the signer. celestia-app v2.x splits a txClient from the signer. See [#3433](https://github.com/celestiaorg/celestia-app/pull/3433). + In celestia-app v1.x, the shares package was included. In v2.x, the [go-square/shares](https://github.com/celestiaorg/go-square/tree/c8242f96a844956f8d1c60e5511104deed8bc361/shares) package is used. + The blob.types package with the `CreateCommitment` function in v1.x is replaced by the [go-square/inclusion](https://github.com/celestiaorg/go-square/tree/c8242f96a844956f8d1c60e5511104deed8bc361/inclusion) function in v2.x. + A lot of functionality included in the signer in v1.x has been split into a txClient in v2.x. See [#3433](https://github.com/celestiaorg/celestia-app/pull/3433).Tools
LanguageTool
[style] ~20-~20: 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] ~20-~20: 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)
This feels a bit duplicative of the information in the actual v2.0.0 release notes. The only thing missing from the v2.0.0 release notes that is present here is the note to enable BBR which wasn't actually included in the v2.0.0 release. |
|
||
Consensus node operators are expected to upgrade to this release _prior_ to the Lemongrass hardfork if they intend to continue participating in the network. The command used to start the [consensus node](https://docs.celestia.org/nodes/consensus-node#start-the-consensus-node) or [validator node](https://docs.celestia.org/nodes/validator-node#run-the-validator-node) will accept an additional `--v2-upgrade-height` flag. See [this table](https://docs.celestia.org/nodes/hardfork-process#lemongrass-hardfork) for upgrade heights for each network. | ||
|
||
Consensus node operators should enable the BBR (Bottleneck Bandwidth and Round-trip propagation time) congestion control algorithm. See [#3812](https://github.com/celestiaorg/celestia-app/pull/3812). |
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.
FYI: @cmwaters
Closes #3791