Increment version by semver keyword with --cd-version flag #607

cif · 2017-02-17T23:53:59Z

We do automated code analysis in our deploy pipelines and have a new Lerna repo which needs to operate in independent version mode. Currently the only way to automate publish (no prompts) from build server CL is to specify --repo-version=x.x.x --yes (or canary), however this forces all modules into that version.

This PR allows setting up any changed "next" version(s) appropriately via root package.json / publish script --cd-version=[patch|minor|major] and pre setting the next value(s) in each module.

When code is approved and subsequently merged, our build pipelines can use the right versions and deploy to our private npm repository.

cif · 2017-02-18T01:38:18Z

test/PublishCommand.js

@@ -204,6 +204,85 @@ describe("PublishCommand", () => {
        }
      }));
    });
+
+    it("should use semver increments when passed to repoVersion flag", (done) => {


Just a suggestion, but feel it would be worth refactoring these tests to smaller assertion blocks / calls rather than the larger fixture > result chunks. CLI tools with lots of arg permutations is hard to test all paths

Sounds interesting, but not sure I understand clearly. Can you give an example of how you'd like to see it organized?

gigabo · 2017-02-18T01:47:16Z

Oh, wow. This is great! Thanks @cif!

I think this addresses #461.

gigabo · 2017-02-18T02:01:29Z

src/commands/PublishCommand.js

+        });
+
+        return callback(null, { versions });
+      }


Hmm... actually I wonder if --repo-version is the right option for this. That's really meant specifically for non-independent versioning. And actually looking at this again it will only work correctly for independently versioned repos. It won't update the version in lerna.json unless the return value from getVersionsForUpdates includes a version (singular) value.

It might actually be better to add a new option that works for both independent and non-independent repos.

I had the same thoughts :) This was the easiest point of interception to get this working locally without introducing much new functionality. Stoked maintainers are willing to help guide this one, we really need it. Any suggestions on flag name? I feel it should be fairly straight forward within the same method to intercept and alter the test. I'll get back to you with more suggestions on that first thing next week. As I stated in the comment, definitely tough to get all paths (especially the inverse coverage) when you have many options

gigabo

Got a little too excited when I first saw this. 😁

A few requests (more detail here):

I'm not sure this should re-use --repo-version.
I'd like to be able to use it in non-independent repos.
It would be nice to have some mention of it in the README!

cif · 2017-02-20T19:50:44Z

I made all the changes requested and made the corresponding tests match more closely with your current pattern. Unfortunately, I can't figure out why Appveyor is upset. I changed the ci script to remove all the verbose logging to see if I could isolate any one problematic test. All tests are working locally, Travis is happy but Appveyor is timing out:

182 passing (38s)
  1 failing
  1)  "after all" hook:
     Error: timeout of 5000ms exceeded. Ensure the done() callback is being called in this test.
      at done (C:\projects\lerna\node_modules\mocha\lib\runnable.js:285:13)
      at callFn (C:\projects\lerna\node_modules\mocha\lib\runnable.js:344:7)
      at Hook.Runnable.run (C:\projects\lerna\node_modules\mocha\lib\runnable.js:319:7)
      at next (C:\projects\lerna\node_modules\mocha\lib\runner.js:298:10)
      at Immediate.<anonymous> (C:\projects\lerna\node_modules\mocha\lib\runner.js:320:5)

What's interesting, is that there are 182 tests currently, so the mysterious fail doesn't seem to exist at all?

…b.com/cif/lerna into feature/repo-version-semver-increment

cif · 2017-02-21T17:18:34Z

package.json

-    "test": "mocha -t 5000",
-    "ci": "npm run lint && cross-env DEBUG_CALLS=true npm run test",
+    "test": "mocha -t 20000",
+    "ci": "npm run lint && cross-env npm run test",


Had to increase timeout for appveyor tests to pass (see main conversation thread for error there). Also removed DEBUG_CALLS to make test output readable. If you want it restored, happy to do as part of this PR

Seems fine to me. We removed it in Asini, too.

We could actually also remove the cross-env, too, since that was just for getting the environment variable set consistently across environments. Then we could drop the dep, I think.

@hzoo @evocateur @doug-wade Any objection to dropping this?

gigabo · 2017-02-21T18:49:16Z

src/commands/PublishCommand.js

+        // Use the cdVersion flag to bump the global version as well
+        const version = this.flags.cdVersion === "current" ?
+          this.globalVersion : semver.inc(this.globalVersion, this.flags.cdVersion);
+        return callback(null, { versions, version });


Looking at the caller again I think in independent mode version (singular) is expected not to be set. So this should either set versions (independent) or set version (not). Kind of a weird interface. 😖

I was hesitant to send both to callback in the deconstructed object, however in independent mode updateVersionInLernaJson method isn't called due to execute:

if (!this.repository.isIndependent() && !this.flags.canary) { this.updateVersionInLernaJson(); }

I did local tests switching between both and publishing to our local npm repo. I also added both normal and independent modes to the test suite.

If it's more clear, we can use another !this.repository.isIndependent() check here and send either-or to callback. Just let me know and consider it done if that's the call

I went ahead and made the change to getVersionsForUpdates() so the object passed is explicitly version or versions

cif · 2017-02-23T19:00:25Z

src/NpmUtilities.js

@@ -76,21 +76,21 @@ export default class NpmUtilities {
  }

  @logger.logifySync()
-  static addDistTag(packageName, version, tag, registry) {
+  static addDistTag(directory, packageName, version, tag, registry) {


I was getting odd NPM ERR on the final publish step in my build and realized because our modules are all @scoped/module in the package.json but simply module in the directory that tagging would only work if you cd into the module before tagging

How about specifying the cwd option for execSync rather than the manual cd?

I changed getTagOpts signature to accept directory in addition to registry. publishTaggedInDir was already using the same cd ... && npm ... method. I updated this so that all methods are now using your suggestion

evocateur · 2017-02-27T01:49:54Z

src/NpmUtilities.js

@@ -76,21 +76,21 @@ export default class NpmUtilities {
  }

  @logger.logifySync()
-  static addDistTag(packageName, version, tag, registry) {
+  static addDistTag(directory, packageName, version, tag, registry) {


How about specifying the cwd option for execSync rather than the manual cd?

evocateur · 2017-02-27T01:53:39Z

src/commands/PublishCommand.js

+          const versions = {};
+          this.updates.forEach((update) => {
+            versions[update.package.name] = this.flags.cdVersion === "current" ?
+              update.package.version : semver.inc(update.package.version, this.flags.cdVersion);


Can you put the ternary operators first, please? It is much easier to read, in my experience.

versions[update.package.name] = (this.flags.cdVersion === "current") ? update.package.version : semver.inc(update.package.version, this.flags.cdVersion);

Ternaries no longer needed due to current version removed

evocateur · 2017-02-27T01:55:48Z

src/commands/PublishCommand.js

+          this.flags.cdVersion === "major" ||
+          this.flags.cdVersion === "current"
+      ) {
+        // Warn about possible problems with git and current


What is the use-case for --cd-version current?

Kind of a long story having to do with our deployment pipeline but we came to the conclusion it was a better strategy to always version. Removing the option in latest changes

nutgaard · 2017-03-02T07:50:12Z

Is there anything stopping this from getting merged at this point? Anything I can help with?
I would really really like to see this feature in lerna. :)

cif · 2017-03-02T16:14:35Z

There's a bit of talk in #567 about how to support the other semvers e.g. prerelease as well as release phase/scope (rc | beta | ...). Should all be fairly straight forward updates to PublishCommand.js, IMO wouldn't have to be part of this PR but that's up to the maintainers :)

omerxx · 2017-03-09T16:06:03Z

Hey @cif / @evocateur,
This feature would help us a lot, saving time and effort, and solve the problem very neatly.
Is there a forecast on when this should be ready and merged into lerna?

Thank you guys for the hard work!

evocateur · 2017-03-09T18:29:28Z

@gigabo Are you satisfied with this now?

I'll look into the conflicts today.

gigabo · 2017-03-09T18:33:29Z

Yep, looks good. Thanks @cif!

cif · 2017-03-09T19:27:42Z

@evocateur Just took a swing at resolving them, waiting on tests will update if they fail master was still assuming that call exec were using "cd ..." whereas the changes you suggested eliminated that need (use cwd opt)

evocateur · 2017-03-09T19:31:15Z

@cif thanks, I appreciate it. I'm also in the process of rebasing, and so far I've found that upstream changes make it really tricksy...

evocateur · 2017-03-09T19:32:42Z

in test/PublishCommand.js, the expectedOpts stuff should really be expectedEnv, allowing the cwd to change correctly in the stub assertions. I'm almost there.

omerxx · 2017-03-09T20:06:04Z

Wow! You guys are awesome... Thank you for this!

cif · 2017-03-10T00:19:10Z

@evocateur sounds like we may have overlapped, but I just pushed up some quick fixes for those tests. Feel free to clean up, could probably have used a helper instead of the copy/paste

evocateur · 2017-03-10T02:03:39Z

Thanks @cif !

bradbarrow · 2017-07-06T13:51:53Z

I'd love to better understand how this is used in your CD process @cif

If I understand, you'd have to run lerna publish --yes --cd-version=minor for instance which would then force all of the packages to be bumped a minor bump?

Is that the behaviour?

What happens if in a single CI build I want one package to be published in a major increment and one in a minor?

cif · 2017-07-06T17:56:56Z

@bradbarrow We only increment the same value for each package. In our case we only use independent versioning because our starting points were already different which is why I created the PR.

To handle the use case you describe it would require code changes as well as a specification for each increment on a package level (perhaps in each package.json) which seems tedious to maintain especially if you have a lot of packages.

On a higher/philosophical level, it's worth asking the question - if your packages are versioning semantically independent of each other, are they really fit to be contained in a mono-repo project or are they better as independently maintained projects at that point?

Generally, most of the teams I see who have the choice of lockstep vs independent (i.e. Babel https://github.com/babel/babel/blob/7.0/lerna.json) will choose lockstep if possible.

Hope that helps!

lock · 2018-12-27T13:10:22Z

This thread has been automatically locked because there has not been any recent activity after it was closed. Please open a new issue for related bugs.

Ben Ipsen added 3 commits February 17, 2017 14:00

Flag —-repo-version can be passed semver increment

f05e704

Option for current version, pra/merge pipelines

b0abdb4

Fixed test using EOL for NT environment appveyor

f1f031f

cif commented Feb 18, 2017

View reviewed changes

gigabo added the Tag: Enhancement label Feb 18, 2017

gigabo approved these changes Feb 18, 2017

View reviewed changes

gigabo reviewed Feb 18, 2017

View reviewed changes

gigabo suggested changes Feb 18, 2017

View reviewed changes

Ben Ipsen added 4 commits February 20, 2017 11:00

Update to use —-cd-version, README, tests

916f329

Removed necessary check due to git warning

b8f2ef3

Quick change to README for appveyor build retry

4906409

Removed debug from ci script

6f73862

Ben Ipsen added 6 commits February 21, 2017 08:42

Removing new cd version tests for appveyor

cef1634

Full revert to master /test dir

1aec9b8

Revert publish command to master appveyor test

fb6d2cb

Increasing timeout on mocha per appveyor forum

762a33b

Merge branch 'feature/repo-version-semver-increment' of https://githu…

bac6568

…b.com/cif/lerna into feature/repo-version-semver-increment

Restored changes

88cf53b

cif commented Feb 21, 2017

View reviewed changes

cif changed the title ~~Feature: Independent versioning increments via --repo-version semver value~~ Feature: Independent versioning increments via --cd-version semver value Feb 21, 2017

gigabo reviewed Feb 21, 2017

View reviewed changes

Ben Ipsen and others added 4 commits February 22, 2017 08:18

Change callback argument to explicit version mode

9f678d2

Remove additinoal line break

0919778

Have to cd into directories to add / rm tags

41f5d9d

Merge branch 'master' into feature/repo-version-semver-increment

addbf8d

cif commented Feb 23, 2017

View reviewed changes

Fixed tests from changes to master

254e679

evocateur suggested changes Feb 27, 2017

View reviewed changes

cif added 2 commits February 28, 2017 16:41

Merge branch 'master' into feature/repo-version-semver-increment

109356c

Fix broken test from master merge conflict

33f555d

nutgaard mentioned this pull request Mar 9, 2017

Is there a way to automatically select patch on all lerna publish version selection prompts? #659

Closed

gigabo approved these changes Mar 9, 2017

View reviewed changes

Merge branch 'master' into feature/repo-version-semver-increment

f564d43

Fixed up tests for cwd option

403a326

Ben Ipsen and others added 5 commits March 9, 2017 16:24

Oops lint didnt work in editor

0a8d213

cleanups

1ed379f

don't tweak env before magic hidden async operations complete :P

dda2c02

remove unused cross-env

6b33324

remove references to DEBUG_CALLS env var

f385531

evocateur changed the title ~~Feature: Versioning increments via --cd-version semver value~~ Add --cd-version flag for incrementing version by semver keyword Mar 10, 2017

evocateur changed the title ~~Add --cd-version flag for incrementing version by semver keyword~~ Increment version by semver keyword with --cd-version flag Mar 10, 2017

evocateur merged commit 25d3442 into lerna:master Mar 10, 2017

cif deleted the feature/repo-version-semver-increment branch July 6, 2017 17:57

lock bot locked as resolved and limited conversation to collaborators Dec 27, 2018

Increment version by semver keyword with --cd-version flag #607

Increment version by semver keyword with --cd-version flag #607

Conversation

cif commented Feb 17, 2017 • edited by evocateur

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gigabo commented Feb 18, 2017

Choose a reason for hiding this comment

cif Feb 18, 2017 • edited

Choose a reason for hiding this comment

gigabo left a comment • edited

Choose a reason for hiding this comment

cif commented Feb 20, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cif Feb 21, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nutgaard commented Mar 2, 2017

cif commented Mar 2, 2017

omerxx commented Mar 9, 2017

evocateur commented Mar 9, 2017

gigabo commented Mar 9, 2017

cif commented Mar 9, 2017

evocateur commented Mar 9, 2017

evocateur commented Mar 9, 2017

omerxx commented Mar 9, 2017

cif commented Mar 10, 2017 • edited

evocateur commented Mar 10, 2017

bradbarrow commented Jul 6, 2017

cif commented Jul 6, 2017 • edited

lock bot commented Dec 27, 2018

cif commented Feb 17, 2017 •

edited by evocateur

cif Feb 18, 2017 •

edited

gigabo left a comment •

edited

cif Feb 21, 2017 •

edited

cif commented Mar 10, 2017 •

edited

cif commented Jul 6, 2017 •

edited