-
Notifications
You must be signed in to change notification settings - Fork 492
fix: "prerelease" versioning strategy #2516
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
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
This is actually a bugfix I need as well! |
|
@chingor13 would like to see this get merged. |
|
@chingor13 would like to see this get merged. |
|
@ferrarimarco do you think you could review this? |
ferrarimarco
left a comment
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 for this PR.
A few small things to check.
|
|
||
| # Devenv | ||
| .devenv* | ||
| devenv.local.nix | ||
| devenv.lock | ||
| devenv.nix | ||
| devenv.yaml | ||
| .envrc | ||
|
|
||
| # direnv | ||
| .direnv | ||
|
|
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 you please revert this change? we don't use devenv.
| | `always-bump-major` | Always bump major version | | ||
| | `service-pack` | Designed for Java backport fixes. Uses Maven's specification for service pack versions (e.g. 1.2.3-sp.1) | | ||
| | `prerelease` | Bumping prerelease number (eg. 1.2.0-beta01 to 1.2.0-beta02) or if prerelease type is set, using that in the prerelease part (eg. 1.2.1 to 1.3.0-beta) | | ||
| | `prerelease` | Bumping prerelease number (eg. 1.2.0-beta01 to 1.2.0-beta02) or if prerelease type is set, using that in the prerelease part (eg. 1.2.1 to 1.3.0-beta). Works together with the "prerelease" settings from [manifest-releaser](/docs/manifest-releaser.md) (see for more infos) - a prerelease version number will only be created, if the prerelease setting is set to true (false by default). | |
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 you please refactor to:
A prerelease version number will only be created if the prerelease setting is set to `true`. Default: `false`. For more information, see [Manifest Driven release-please](manifest-releaser.md).
| // Works together with the "prerelease" versioning strategy, which creates a | ||
| // prerelease-version (like 1.0.0-alpha.1) only if this setting is set to true. | ||
| // This allows to create prerelease-versions on conditions (e.g. publish them only | ||
| // on release-candidate branches) |
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 you please extend this comment to explain the use case that led to this fix, so that other users are aware of how to work with pre-releases when you want to release a non pre-release version?
| describe('without prerelease property', () => { | ||
| const expectedBumps: Record<string, string> = { | ||
| '1.2.3': '2.0.0', | ||
| '1.0.0-beta01': '1.0.0', |
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 you add a comment explaining why 1.0.0 here? So we don't lose context.
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes ##2515 🦕