-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
feat: preserve major version zero on breaking changes (when using conventional commits) #2486
Conversation
According to semver, major version zero (0.y.z) is for initial development. Anything MAY change at any time. The public API SHOULD NOT be considered stable. The version 1.0.0 defines the (initial stable) public API. To allow monorepos to use major version zero meaningfully, the transition from 0.x to 1.x must be explicitly requested by the user. Breaking changes MUST NOT automatically bump the major version from 0.x to 1.x. The usual convention is to use semver-patch bumps for bugfix releases and semver-minor for everything else, including breaking changes. This matches the behavior of `^` operator as implemented by `npm`. This commit implements the convention described above: - a patch-level change bumps semver-patch version (NO CHANGE) - a minor-level change bumps semver-minor version (NO CHANGE) - a major-level (breaking) change bumps semver-minor version (NEW) Signed-off-by: Miroslav Bajtoš <mbajtoss@gmail.com>
Discussion points:
|
// as implemented by `npm`. | ||
// | ||
if (releaseType === "major") { | ||
releaseType = "minor"; |
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 guess if --conventional-graduate
is true
, we will bump the release to the new major, such as 0.1.0
-> 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.
Personally, I would find such rule as possibly confusing. Also, how would such rule work in large monorepos having packages both in pre-release versions (2.0.0-beta.1
) and major zero versions (0.2.5
)?
I am currently envisioning the following process for graduating from 0.x to 1.0.0:
-
Add a commit changing the version in package.json to
1.0.0-1
(or a similar pre-release). This will be just a commit, no release/tag is associated with it. The commit can have a nice descriptive message for the changelog, e.g.feat: graduate to 1.0
. -
Run
lerna version
with--conventional-graduate
to bump the temporary version1.0.0-1
to final1.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.
Please note --conventional-graduate
allows a list of packages. Whether it's passed in as a CLI option or configured in lerna.json
, we probably should honor it. See https://github.com/lerna/lerna/blob/master/commands/version/README.md#--conventional-graduate.
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 don't see --conventional-graduate
affecting 0.x ranges, as they are not "prereleases" in the sense that option is intended to modify. 0.x ranges, as I understand them, mean "API unstable", not "this is a prerelease of a specified quality".
@bajtos please refer to #2549 I've just created. NPM treats minor change to 0.x versions as breaking, ie. will not update 0.4 to 0.5. This can be tried on https://semver.npmjs.com Thus, for 0.x packages only breaking change should bump minor and any other change should bump patch |
Thank you @tpluscode for the link to the issue you have created. I am confused about why are you posting this here? My pull request have been waiting for review by project maintainers for more than a month by now. I don't see how filling a new issue is going to help progress forward? Are you perhaps trying to say that my pull request is implementing incorrect logic for determining which part of the 0.x version to bump up? If yes, then could you please be more specific about what should I change? |
I'm sorry, I thought you would tell me. I was only basing my comment on the description and not code. Here is where our understanding differs on incrementing 0.x packages. You say
Where I think it should be
I say that because npm treats minor bump on 0.x as breaking change and will not update automatically. I will however update for example |
Thank you @tpluscode for clarification!
You have raised a valid point 👍 Personally, I don't care that much whether new features bump semver-minor or semver-patch version. I am fine to implement whichever rule that gets widest consensus. Anything would be better than the current behavior, where breaking changes are bumping 0.x to 1.0 :( |
Agreed. This is why I create a separate issue. In the event that your PR gets merged at its current state and it'd still need tweaking to align 100% with npm's treatment of 0.x version. Unless you'd agree to adjust right here and we can kill two birds with one stone |
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 not a huge fan of the 0.x range for many reasons, but I agree we should adhere closer to how npm deals with those ranges for a more congruent developer experience.
// the (initial stable) public API. | ||
// | ||
// To allow monorepos to use major version zero meaningfully, | ||
// the transition from 0.x to 1.x must be explicitly requested |
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 some info-level logging indicating what is happening would be helpful. I appreciate the detailed comment.
// as implemented by `npm`. | ||
// | ||
if (releaseType === "major") { | ||
releaseType = "minor"; |
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 don't see --conventional-graduate
affecting 0.x ranges, as they are not "prereleases" in the sense that option is intended to modify. 0.x ranges, as I understand them, mean "API unstable", not "this is a prerelease of a specified quality".
Thanks for your patience! |
@evocateur thank you for landing my pull request! ❤️ |
it includes what we were patching: lerna/lerna#2486
it includes what we were patching: lerna/lerna#2486
it includes what we were patching: lerna/lerna#2486
Description
According to semver, major version zero (
0.y.z
) is for initial development. Anything MAY change at any time. The public API SHOULD NOT be considered stable. The version1.0.0
defines the (initial stable) public API.In my opinion formed by my experience building https://github.com/strongloop/loopback-next, in order to allow monorepos to use major version zero meaningfully, the transition from
0.x
to1.x
must be explicitly requested by the user. Breaking changes MUST NOT automatically bump the major version from0.x
to1.x
.The usual convention is to use semver-patch bumps for bugfix releases and semver-minor for everything else, including breaking changes. This matches the behavior of
^
operator as implemented bynpm
(see docs of the semver module).This commit implements the convention described above for packages in major version zero:
Motivation and Context
In LoopBack, we have been several times bitten by
lerna version
bumping up our packages from 0.x to 1.0 (see loopbackio/loopback-next#1068). Most recently, we dropped support for Node.js 8.x because it reached end of life. We described this change as breaking, and so in the next release, lerna wanted to graduate our experimental packages with unstable API to stable 1.0. That was not desired at all!Related discussions and alternative solutions:
How Has This Been Tested?
I added a new unit test, verified that it fails against the current implementation and passes with my patch applied.
I am not sure if a single test is enough for a change like this. Please suggest what other scenarios to cover by tests to give us enough confidence, I am happy to add more tests as needed.
Types of changes
Checklist:
lerna bootstrap --npm-client yarn
is failing for me, the error seems unrelated to my changes.)