Use --canary=<value> as prerelease tag, not commit-ish #1020

achingbrain · 2017-09-20T15:40:57Z

Description

The docs say we should be able to override the meta suffix by passing the canary flag as a string value. lerna takes this value and tries to use it to work out what range of commits it should consider when publishing a package so you can't actually override the meta suffix unless you use a commit id to do this. Considering the commit id is added to the version number for canary releases anyway this seems redundant.

eg:

This works:

lerna publish --canary
...
Changes:
 - package-1: 1.0.0 => 1.1.0-alpha.0a2f1a97
 - package-1: 1.0.0 => 1.1.0-alpha.0a2f1a97

According to the docs this should work but doesn't:

lerna publish --canary=beta
...
lerna ERR! initialize Error: fatal: bad revision 'beta^..beta'
*kaboom!*

Motivation and Context

It would be useful to be able to override the meta suffix as described in the docs.

Fixes #1008.

How Has This Been Tested?

Added some unit tests for the UpdatedPackagesCollector module and integration tests for the canary flag.

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 have updated the documentation accordingly.
I have read the CONTRIBUTING document.
I have added tests to cover my changes.
All new and existing tests passed.

achingbrain · 2017-09-21T15:17:55Z

Hmm, the appveyor builds seem to fail in different ways each time, none of which seem to be related to my changes.

GeorgeTaveras1231 · 2017-09-25T01:53:37Z

@achingbrain was going to work on this myself but glad to see you got this.

Additionally, I wanted to raise a concern with the lerna maintainers about the role that the canary option value is playing and whether it also needs to be used as the pre-id, even though there is already a flag to define the pre-id. It's not a huge deal but it just feels inconsistent and my response was 🤔 when I realized I could not use --pre-id for this.

achingbrain · 2017-09-26T13:20:48Z

I see what you mean. That said canary works well for my current project which requires per-commit automated CI deployment for internal testing with a manual public release gate.

@evocateur any thoughts on this PR?

evocateur · 2017-09-26T16:48:19Z

I appreciate the PR, @achingbrain. I am not generally a fan of "polymorphic" options that we inherited from the more lenient meow cli parser. I would much prefer --preid to be used for this purpose, as @GeorgeTaveras1231 suggested (--canary would always be a boolean, never a string).

--preid's implicit default is "alpha", which is exactly what the (implicit :P) default of --canary-as-string is now. I think that's the long-term solution. In the mean-time, this PR seems reasonable. It's not a breaking change, imo, because the functionality never worked as described during the non-beta 2.x cycle to date.

evocateur

Thanks for your patience, @achingbrain, just a few small fixes and we can get this merged.

evocateur · 2017-09-26T16:51:51Z

test/UpdatedPackagesCollector.js

+const repository = {
+  rootPath: 'root-path'
+};
+
 describe("UpdatedPackagesCollector", () => {
  it("should exist", () => {
    expect(UpdatedPackagesCollector).toBeDefined();
  });

  it("needs better tests");


It now has them, thank you! (you can delete this empty test case, if you want)

Heh, I left that in because I agree with the sentiment 😉

Will remove.

evocateur · 2017-09-26T16:53:28Z

test/integration/lerna-publish.test.js

@@ -13,6 +13,9 @@ import loadPkgManifests from "../helpers/loadPkgManifests";
 const lastCommitMessage = (cwd) =>
  execa.stdout("git", ["log", "-1", "--format=%B"], { cwd }).then(normalizeNewline);

+const lastCommitId = (cwd) =>
+  execa.stdout("git", ["rev-parse", "HEAD"], { cwd }).then((line) => line.trim().substring(0, 8));


I thought execa automatically trimmed stdout?

evocateur · 2017-09-26T16:54:48Z

test/integration/lerna-publish.test.js

+    const { stdout, stderr } = await execa(LERNA_BIN, args, { cwd });
+    const hash = await lastCommitId(cwd);
+    expect(stdout.replace(new RegExp(hash, 'g'), 'hash')).toMatchSnapshot("stdout: canary default version");
+    expect(stderr).toMatchSnapshot("stderr: canary beta version");


This snapshot label should be stderr: canary default version, looks like copy+paste oversight.

achingbrain · 2017-10-02T13:56:07Z

@evocateur I've made the changes as requested, is there anything else that needs doing before this can be merged? Thanks..

evocateur · 2017-10-02T16:36:09Z

@achingbrain Thanks!

lock · 2018-12-27T12:09:49Z

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.

Fixes #1008 by not using the passed canary flag as a commit id

9974238

GeorgeTaveras1231 mentioned this pull request Sep 25, 2017

Allow --since flag on publish --canary #963

Closed

9 tasks

evocateur suggested changes Sep 26, 2017

View reviewed changes

Addresses PR comments

f791909

evocateur approved these changes Oct 2, 2017

View reviewed changes

evocateur added the Tag: Bug Fix label Oct 2, 2017

evocateur changed the title ~~Fixes #1008 by not using the passed canary flag as a commit id~~ Use --canary=<value> as prerelease tag, not commit-ish Oct 2, 2017

evocateur merged commit 5c079a5 into lerna:master Oct 2, 2017

dependencies bot mentioned this pull request Oct 3, 2017

Update lerna from 2.2.0 to 2.3.1 in / storybookjs/storybook#1954

Merged

evocateur mentioned this pull request Nov 15, 2017

publish --canary should have some way of limiting publish to changed packages only #962

Open

dependencies bot mentioned this pull request Nov 17, 2017

Update lerna in / from 2.2.0 to 2.5.1 src-zone/material#17

Closed

renovate bot mentioned this pull request Feb 2, 2018

chore(deps): update dependency lerna to v2.9.0 TylorS/typed-unmaintained#47

Closed

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use --canary=<value> as prerelease tag, not commit-ish #1020

Use --canary=<value> as prerelease tag, not commit-ish #1020

achingbrain commented Sep 20, 2017 •

edited

achingbrain commented Sep 21, 2017

GeorgeTaveras1231 commented Sep 25, 2017

achingbrain commented Sep 26, 2017

evocateur commented Sep 26, 2017

evocateur left a comment

evocateur Sep 26, 2017

achingbrain Sep 27, 2017

evocateur Sep 26, 2017

achingbrain Sep 27, 2017

evocateur Sep 26, 2017

achingbrain Sep 27, 2017

achingbrain commented Oct 2, 2017

evocateur commented Oct 2, 2017

lock bot commented Dec 27, 2018

Use --canary=<value> as prerelease tag, not commit-ish #1020

Use --canary=<value> as prerelease tag, not commit-ish #1020

Conversation

achingbrain commented Sep 20, 2017 • edited

Description

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

achingbrain commented Sep 21, 2017

GeorgeTaveras1231 commented Sep 25, 2017

achingbrain commented Sep 26, 2017

evocateur commented Sep 26, 2017

evocateur left a comment

Choose a reason for hiding this comment

evocateur Sep 26, 2017

Choose a reason for hiding this comment

achingbrain Sep 27, 2017

Choose a reason for hiding this comment

evocateur Sep 26, 2017

Choose a reason for hiding this comment

achingbrain Sep 27, 2017

Choose a reason for hiding this comment

evocateur Sep 26, 2017

Choose a reason for hiding this comment

achingbrain Sep 27, 2017

Choose a reason for hiding this comment

achingbrain commented Oct 2, 2017

evocateur commented Oct 2, 2017

lock bot commented Dec 27, 2018

achingbrain commented Sep 20, 2017 •

edited