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

deps: update npm to 6.12.0 #29885

Closed
wants to merge 1 commit into from
Closed

deps: update npm to 6.12.0 #29885

wants to merge 1 commit into from

Conversation

isaacs
Copy link
Contributor

@isaacs isaacs commented Oct 8, 2019

Update npm to 6.12.0. Release notes:

6.12.0 (2019-10-08):

Now npm ci runs prepare scripts for git dependencies, and respects the --no-optional argument. Warnings for engine mismatches are printed again. Various other fixes and cleanups.

BUG FIXES

FEATURES

  • ed993a29c #249 Add CI environment variables to user-agent (@isaacs)
  • f6b0459a4 #248 Add option to save package-lock without formatting Adds a new config --format-package-lock, which defaults to true. (@bl00mber)

DEPENDENCIES

TESTING

@nodejs-github-bot nodejs-github-bot added the npm Issues and PRs related to the npm client dependency or the npm registry. label Oct 8, 2019
@nodejs-github-bot
Copy link
Collaborator

@sam-github
Copy link
Contributor

@nodejs/npm I kicked off normal PR CI, but don't know if any extra is required for npm. PTAL

@nodejs-github-bot
Copy link
Collaborator

@Trott Trott added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Oct 9, 2019
Copy link
Member

@Trott Trott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rubber-stamp LGTM

Copy link
Member

@trivikr trivikr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RSLGTM

Copy link
Member

@BridgeAR BridgeAR left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RSLGTM

@Trott
Copy link
Member

Trott commented Oct 10, 2019

According to policy, this can land on October 22. However, that's also the day of the Node.js 13.0.0 release. I'd really like to see this in that release, so I'd like to propose shortening the npm-merge wait-period to 1 week for this, which means that it can land on October 15.

Thoughts on this, @nodejs/tsc, @nodejs/releasers, and especially @BethGriggs who is doing the release?

(The Release WG is solely responsible for determining the contents of any release, but they usually ask/defer to the TSC on controversial things, so tagging both.)

@Trott Trott added the notable-change PRs with changes that should be highlighted in changelogs. label Oct 10, 2019
@Trott Trott added this to the 13.0.0 milestone Oct 10, 2019
@BethGriggs
Copy link
Member

I'm +1 on this landing ~15th October so it can make it into the final release candidate of v13.0.0

@mcollina
Copy link
Member

mcollina commented Oct 11, 2019 via email

Trott added a commit to Trott/io.js that referenced this pull request Oct 12, 2019
The two-week wait period for merging npm releases is one of those rule
exceptions that would be great to get rid of (in my opinion at least).
There are too many exceptions to our rules and they tend to be scattered
across multiple documents. People don't feel confident they know the
rules, thus hampering both project velocity and Collaborator confidence.
It also means I (and perhaps others?) get lots of pings about whether
this or that can land, etc.

This particular issue has come up a few times lately, and is
specifically calling for an exception-to-the-exception so that the
latest version of npm can be released along with Node.js 13.0.0.
Refs: nodejs#29885 (comment)

I propose here reducing the wait period from two weeks to one week. If,
after some amount of time, there seems to be no problems caused by this
change, we can consider further reducing the wait period to 48 hours to
align it with all other change requests. Even if you think that is going
too far, hopefully we can at least get it reduced to a week, as the
second week of the waiting period is usually just the PR sitting around
with an occasional ping from someone about whether/when it can land.

PR-URL: nodejs#29922
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Anto Aravinth <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
Copy link
Contributor

@cclauss cclauss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let’s go...

Trott pushed a commit to Trott/io.js that referenced this pull request Oct 15, 2019
Update npm to 6.12.0

Now `npm ci` runs prepare scripts for git dependencies, and respects the
`--no-optional` argument.  Warnings for `engine` mismatches are printed
again.  Various other fixes and cleanups.

PR-URL: nodejs#29885
Reviewed-By: Sam Roberts <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Beth Griggs <[email protected]>
Reviewed-By: Christian Clauss <[email protected]>
@Trott
Copy link
Member

Trott commented Oct 15, 2019

Landed in 3ebaf6b

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. notable-change PRs with changes that should be highlighted in changelogs. npm Issues and PRs related to the npm client dependency or the npm registry.
Projects
None yet
Development

Successfully merging this pull request may close these issues.