move from --skip-temp-tag to --temp-tag #777

noherczeg · 2017-04-22T00:21:17Z

Explicit skipping of a temp tag is no longer necessary since packages are published in topological order. Fixes #742

Description

I removed the --skip-temp-tag flag from the codebase and replaced it with --temp-tag with a default value of false.

This means that if nothing is provided Lerna will not create a temp-tag.

If someone would like to create a temp-tag in the process he/she could do it by providing the --temp-tag flag.

How Has This Been Tested?

Inverted the test case where temp tag creation was disabled and tested if it indeed is created when requested.

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.

noherczeg · 2017-04-22T00:24:15Z

I need help with the wording of the new flag in the docs. I think @hzoo or @evocateur could help out with this. Not sure about keeping the old flag with a deprecation warning is needed either. I put it in just to not confuse devs already using said feature, but it could be removed.

Also need someone to check the updated test snapshots because I'm not 100% certain that everything is okay.

evocateur

Looking good so far!

evocateur · 2017-04-22T15:41:02Z

README.md

@@ -731,11 +731,15 @@ This is useful if you do not want to explicitly set up your registry
 configuration in all of your package.json files individually when e.g. using
 private registries.

-#### --skip-temp-tag
+#### --skip-temp-tag (DEPRECATED)


I think we can just replace --skip-temp-tag with --use-temp-tag, since we're breaking the default behaviour anyway. Even if --skip-temp-tag is passed, it won't matter because that's what we'll be doing anyway.

The description of the flag would be similar to the existing docs, but slightly tweaked:

--use-temp-tag

When passed, this flag will alter the default publish process by first publishing all changed packages to a temporary dist-tag (lerna-temp) and then moving the new version(s) to the default dist-tag (latest).

This is not generally necessary, as Lerna will publish packages in topological order (all dependencies before dependents) by default.

I wanted to use --temp-tag because yargs has a negate-fields feature where one could say --no-temp-tag and that would work better than --no-use-temp-tag.

If this isn't important I can switch to the one you requested.

evocateur · 2017-04-22T15:42:28Z

src/commands/PublishCommand.js

@@ -86,9 +86,11 @@ export const builder = {
    group: "Command Options:",
    describe: "Stop before actually publishing change to npm."
  },
-  "skip-temp-tag": {
+  "temp-tag": {


--use-temp-tag scans a little better, here. Since it's a boolean option, you don't need to provide an explicit default: false since that's what happens anyway when the option is omitted.

evocateur · 2017-04-22T15:47:15Z

src/commands/PublishCommand.js

@@ -521,7 +523,7 @@ export default class PublishCommand extends Command {
  npmPublishAsPrerelease(callback) {
    // if we skip temp tags we should tag with the proper value immediately
    // therefore no updates will be needed
-    const tag = this.options.skipTempTag ? this.getDistTag() : "lerna-temp";
+    const tag = !this.options.tempTag ? this.getDistTag() : "lerna-temp";


I'd prefer to reverse this ternary, for clarity.

const tag = this.options.useTempTag ? "lerna-temp" : this.getDistTag();

evocateur · 2017-04-22T15:51:59Z

test/PublishCommand.js

-          expect(NpmUtilities.checkDistTag).not.toBeCalled();
-          expect(NpmUtilities.removeDistTag).not.toBeCalled();
-          expect(NpmUtilities.addDistTag).not.toBeCalled();
+          expect(NpmUtilities.checkDistTag).lastCalledWith(


These lastCalledWith()s should be replaced by a expect(addedDistTagInDirectories(testDir)).toMatchSnapshot("[normal --use-temp-tag] npm dist-tag add"); assertion.

evocateur · 2017-04-22T15:55:49Z

test/__snapshots__/PublishCommand.js.snap

@@ -1,13 +1,6 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP

-exports[`[independent --canary --npm-tag=next --yes --exact] npm dist-tag add 1`] = `


This snapshot should be replaced by

expect(publishedTagInDirectories(testDir)).toMatchSnapshot("[independent --canary --npm-tag=next --yes --exact] npm publish --tag");

Likewise with the other snapshots that have turned into empty objects, those aren't very useful and should be replaced by publishedTagInDirectories() where it makes sense.

Think I updated everything in #4ed5845, dunno why github isn't detecting it.

evocateur · 2017-04-22T20:47:38Z

Thanks @noherczeg!

cary-smith · 2017-05-03T15:41:29Z

Thank you for making this the default behavior! Just started using lerna and ran into this issue with our own internal npm via Nexus.

lock · 2018-12-27T14:21:15Z

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.

move from --skip-temp-tag to --temp-tag with a default of false

6331247

noherczeg requested review from evocateur and hzoo April 22, 2017 00:21

evocateur reviewed Apr 22, 2017

View reviewed changes

evocateur added the Tag: Breaking Change label Apr 22, 2017

evocateur added this to the v2.0.0 milestone Apr 22, 2017

noherczeg added 2 commits April 22, 2017 19:18

align to requests

1f13d07

align to requests, again

4ed5845

evocateur merged commit 2bcd182 into lerna:master Apr 22, 2017

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

move from --skip-temp-tag to --temp-tag #777

move from --skip-temp-tag to --temp-tag #777

noherczeg commented Apr 22, 2017 •

edited

Loading

noherczeg commented Apr 22, 2017

evocateur left a comment

evocateur Apr 22, 2017 •

edited

Loading

noherczeg Apr 22, 2017 •

edited

Loading

evocateur Apr 22, 2017

evocateur Apr 22, 2017

evocateur Apr 22, 2017

evocateur Apr 22, 2017

evocateur Apr 22, 2017

noherczeg Apr 22, 2017

evocateur commented Apr 22, 2017

cary-smith commented May 3, 2017

lock bot commented Dec 27, 2018

		@@ -1,13 +1,6 @@
		// Jest Snapshot v1, https://goo.gl/fbAQLP

		exports[`[independent --canary --npm-tag=next --yes --exact] npm dist-tag add 1`] = `

move from --skip-temp-tag to --temp-tag #777

move from --skip-temp-tag to --temp-tag #777

Conversation

noherczeg commented Apr 22, 2017 • edited Loading

Description

How Has This Been Tested?

Types of changes

Checklist:

noherczeg commented Apr 22, 2017

evocateur left a comment

Choose a reason for hiding this comment

evocateur Apr 22, 2017 • edited Loading

Choose a reason for hiding this comment

--use-temp-tag

noherczeg Apr 22, 2017 • edited Loading

Choose a reason for hiding this comment

evocateur Apr 22, 2017

Choose a reason for hiding this comment

evocateur Apr 22, 2017

Choose a reason for hiding this comment

evocateur Apr 22, 2017

Choose a reason for hiding this comment

evocateur Apr 22, 2017

Choose a reason for hiding this comment

evocateur Apr 22, 2017

Choose a reason for hiding this comment

noherczeg Apr 22, 2017

Choose a reason for hiding this comment

evocateur commented Apr 22, 2017

cary-smith commented May 3, 2017

lock bot commented Dec 27, 2018

noherczeg commented Apr 22, 2017 •

edited

Loading

evocateur Apr 22, 2017 •

edited

Loading

noherczeg Apr 22, 2017 •

edited

Loading