-
Notifications
You must be signed in to change notification settings - Fork 548
docs: add details for experimental releases #2946
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThis update introduces a new guideline to the Changes
Suggested reviewers
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
🪧 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 (1)
CONTRIBUTING.md (1)
258-259
: Clear Enhancement for Experimental Release TaggingThe new guideline specifying that experimental release git tags should include both the version number and the feature name (with an illustrative example for the MPT beta release) makes the process more explicit. To further improve clarity, consider adding a brief note explicitly stating that this convention applies only to experimental releases, contrasting it with the standard tagging for stable releases.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
CONTRIBUTING.md
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: snippets (22.x)
- GitHub Check: snippets (20.x)
- GitHub Check: snippets (18.x)
- GitHub Check: unit (20.x)
- GitHub Check: browser (18.x)
@@ -255,6 +255,8 @@ This should almost always be done using the [`xrpl-codec-gen`](https://github.co | |||
NOW YOU HAVE PUBLISHED! But you're not done; we have to notify people! | |||
|
|||
1. Run `git tag <tagname> -m <tagname>`, where `<tagname>` is the new package and version (e.g. `[email protected]`), for each version released. | |||
|
|||
For experimental releases, the git tagname should include its own version number along with feature name. Here's an example for [MPT beta release](https://github.com/XRPLF/xrpl.js/releases/tag/xrpl%401.0.1-mpt-beta). |
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'm trying to understand the differences between a usual release and an experimental release. Why was the MPT feature not release with the normal semver nomenclature? Why do we need to do experimental releases at all?
Should we follow the rc-1, 2, ...
system of "release candidate" names instead?
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.
It prevents disruption of the version of a normal release. For example, normal version is 2.4.0, we add an experimental feature with the version 2.5.0-beta. However, if we decide to do a normal release without the experimental feature being 2.5.0, then it wouldn't be accurate. That's why it's better for the feature to use its own version.
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'm still confused. After the release of the experimental feature (titled, say 2.5.0-mpt
), what would be the next version? How do we indicate the continuity of the versioning system? Suppose the next release is 2.5.1
, users do not know if mpt
feature is included in the 2.5.1
release.
Why do we need to consider experimental features? A feature will either be included (if it gains 80% UNL majority vote on the mainnet for 2 weeks) or not. Since this is a binary decision, why should we account for an "experimental feature" ? In this example, the MPT
(or) AMM
feature would definitely be enabled on the XRPL Mainnet within a pre-determined time frame.
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.
The 2.5.0 is a counter-example. An experimental feature would have its own version like the MPT example that's in this PR.
There have been cases where experimental features (AMM, Sidechains) would take awhile to develop such that we would do incremental beta releases for them. They usually would have their own devnets. In the meantime, we would still do the normal releases with other features/fixes. Therefore, having separate versions fixes this issue.
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 fine to have experimental feature with its own version in case the feature will not be on the next release
@@ -255,6 +255,8 @@ This should almost always be done using the [`xrpl-codec-gen`](https://github.co | |||
NOW YOU HAVE PUBLISHED! But you're not done; we have to notify people! | |||
|
|||
1. Run `git tag <tagname> -m <tagname>`, where `<tagname>` is the new package and version (e.g. `[email protected]`), for each version released. | |||
|
|||
For experimental releases, the git tagname should include its own version number along with feature name. Here's an example for [MPT beta release](https://github.com/XRPLF/xrpl.js/releases/tag/xrpl%401.0.1-mpt-beta). |
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.
nit: maybe we should mention that the feature name should not be beta or rc to avoid confusion. I will leave it up to you.
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 (1)
CONTRIBUTING.md (1)
253-253
: Minor grammar improvement and optional clarification on feature names.The new instruction is clear and helpful. For improved clarity and correctness, consider changing "along with feature name" to "along with the feature name".
Optionally, to avoid confusion, you may want to add a note that the feature name should not be "beta" or "rc", as suggested in a previous review.
Example revision:
-For experimental releases, the git tagname should include its own version number along with feature name. Here's an example for [MPT beta release](https://github.com/XRPLF/xrpl.js/releases/tag/xrpl%401.0.1-mpt-beta). +For experimental releases, the git tagname should include its own version number along with the feature name (avoid using "beta" or "rc" as the feature name to prevent confusion). Here's an example for [MPT beta release](https://github.com/XRPLF/xrpl.js/releases/tag/xrpl%401.0.1-mpt-beta).🧰 Tools
🪛 LanguageTool
[uncategorized] ~253-~253: You might be missing the article “the” here.
Context: ...clude its own version number along with feature name. Here's an example for [MPT beta r...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
CONTRIBUTING.md
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
CONTRIBUTING.md
[uncategorized] ~253-~253: You might be missing the article “the” here.
Context: ...clude its own version number along with feature name. Here's an example for [MPT beta r...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
High Level Overview of Change
Add details for experimental releases.
Type of Change
Did you update HISTORY.md?
Test Plan