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

[v14.x] deps: upgrade npm to 6.14.11 #37107

Closed
wants to merge 1 commit into from

Conversation

darcyclarke
Copy link
Member

@darcyclarke darcyclarke commented Jan 28, 2021

6.14.11 (2021-01-07)

DEPENDENCIES

DOCUMENTATION

TESTING

@MylesBorins brought it to my attention that #36838 was/is missing the package version bump; I've walked back through our process & queued this net-new PR up - pretty much confirming that there must have been human error during a rebase at some point (apologies on my part there).

Poking @ruyadorno speifically to double/triple check this.

@nodejs-github-bot nodejs-github-bot added npm Issues and PRs related to the npm client dependency or the npm registry. v14.x labels Jan 28, 2021
Copy link
Member

@ruyadorno ruyadorno left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@ruyadorno
Copy link
Member

Thanks @darcyclarke for taking the time to send a fix 😄 cc @nodejs/releasers

@richardlau I know you just recently cut v10.23.2 that included these changes, from what I see here in this PR it just fixes properly bumping the version number in deps/npm/package.json and updating the deps/npm/CHANGELOG.md file so I'm not really sure if it's worth touching v10 again, let me know what you think 😊

Heads up @BethGriggs I think you want to include this in #37074

@nodejs-github-bot
Copy link
Collaborator

@richardlau
Copy link
Member

@ruyadorno I had a quick discussion with @BethGriggs last night about Node.js 10.23.2 and both of us were leaning towards cutting a 10.23.3 as it’s otherwise confusing if the included npm is not accurately reporting its version.

As an aside, this PR should probably be targeting the staging branch, which would probably result in fewer changed files if done right now but for v14.x-staging it would probably make more sense to drop (i.e. rebase out) the previous npm 6.14.11 commit first.

@MylesBorins MylesBorins changed the base branch from v14.x to v14.x-staging January 28, 2021 16:23
@darcyclarke
Copy link
Member Author

Looks like this needs to be rebased against the staging branch. Can update here in a minute*

@MylesBorins
Copy link
Contributor

@BethGriggs @richardlau I've retargeted this against v14.x-staging and it looks like it probably needs to be rebuilt as there is a conflict. It would appear with a quick scan that the update that landed actually only bumped to 6.14.10, so what is shipping in 10.x is in fact accurate, it was the commit that was labelled wrong.

As such I think we need to reland 6.14.11 on both 14 + 12 + 10 (and cut another 10.x). Personally I think it might be simpler to just land the update commit here and then backport to 12 + 10 rather than rebasing out the erroneous commit.. will make backporting simpler. But cool with whatever makes the most sense to folks

@richardlau
Copy link
Member

@MylesBorins #36571 updated to npm 6.14.10 and went out in the January security releases.

@darcyclarke darcyclarke reopened this Jan 28, 2021
@BethGriggs
Copy link
Member

It would appear with a quick scan that the update that landed actually only bumped to 6.14.10, so what is shipping in 10.x is in fact accurate, it was the commit that was labelled wrong.

I think cc6b695 is the commit that went out in v10.x (cherry-picked from #36838), and that appears to have the content of https://github.com/npm/cli/commits/v6.14.11 (for example, the ini update). My understanding is that we've shipped the npm 6.14.11 content labeled as npm 6.14.10.

I'm leaning towards rebasing out the commit from v14.x-staging, and then just landing an updated PR.

@richardlau
Copy link
Member

I think cc6b695 is the commit that went out in v10.x (cherry-picked from #36838),

Correct, #36838 (comment).

@BethGriggs
Copy link
Member

I've rebased the commit out of v14.x-staging - should this PR be reopened?

@BethGriggs
Copy link
Member

ping @nodejs/npm, was this PR intentionally closed? I'm holding off on v14.15.5 so we can get it in

@ruyadorno
Copy link
Member

@BethGriggs I'll follow up and will let you know, thanks!

@ruyadorno
Copy link
Member

@BethGriggs here's the follow up: #37173

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

6 participants