Navigation Menu

Skip to content
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

Merged
merged 1 commit into from May 24, 2020
Merged

feat: preserve major version zero on breaking changes (when using conventional commits) #2486

merged 1 commit into from May 24, 2020

Conversation

bajtos
Copy link
Contributor

@bajtos bajtos commented Mar 9, 2020

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 version 1.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 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 (see docs of the semver module).

This commit implements the convention described above for packages in major version zero:

  • 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)

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

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation. -- I am not sure about this?
  • I have updated the documentation accordingly. -- I think no doc changes are needed.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed. (lerna bootstrap --npm-client yarn is failing for me, the error seems unrelated to my changes.)

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>
@bajtos
Copy link
Contributor Author

bajtos commented Mar 9, 2020

Discussion points:

  • At high-level, is it acceptable to have different bumping behavior for 0.x versions implemented inside lerna?
  • Should we introduce a new CLI option allowing users to choose between the old or the new behavior? We already have similar options --conventional-prerelease and --conventional-graduate, see https://github.com/lerna/lerna/tree/master/commands/version#prerelease
  • How and where to document the new behavior (and the new option if it's introduced)?

// as implemented by `npm`.
//
if (releaseType === "major") {
releaseType = "minor";

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?

Copy link
Contributor Author

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:

  1. 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.

  2. Run lerna version with --conventional-graduate to bump the temporary version 1.0.0-1 to final 1.0.0.

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.

Copy link
Member

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".

@tpluscode
Copy link

@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

@bajtos
Copy link
Contributor Author

bajtos commented Apr 24, 2020

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?

@tpluscode
Copy link

tpluscode commented Apr 24, 2020

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

semver-patch bumps for bugfix releases
semver-minor for everything else, including breaking changes

Where I think it should be

semver-minor for breaking changes
semver-patch for anything else

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 1.1 -> 1.2.

@bajtos
Copy link
Contributor Author

bajtos commented Apr 30, 2020

Thank you @tpluscode for clarification!

Here is where our understanding differs on incrementing 0.x packages. You say

semver-patch bumps for bugfix releases
semver-minor for everything else, including breaking changes

Where I think it should be

semver-minor for breaking changes
semver-patch for anything else

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 1.1 -> 1.2.

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 :(

@tpluscode
Copy link

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

Copy link
Member

@evocateur evocateur left a 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
Copy link
Member

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";
Copy link
Member

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".

@evocateur
Copy link
Member

Thanks for your patience!

@evocateur evocateur merged commit 6126e6c into lerna:master May 24, 2020
@bajtos
Copy link
Contributor Author

bajtos commented May 28, 2020

@evocateur thank you for landing my pull request! ❤️

@bajtos bajtos deleted the feat/preserve-zero-major branch May 28, 2020 10:39
turadg added a commit to Agoric/agoric-sdk that referenced this pull request Feb 23, 2022
it includes what we were patching: lerna/lerna#2486
turadg added a commit to Agoric/agoric-sdk that referenced this pull request Feb 24, 2022
it includes what we were patching: lerna/lerna#2486
turadg added a commit to Agoric/agoric-sdk that referenced this pull request Feb 24, 2022
it includes what we were patching: lerna/lerna#2486
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants