FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

deps: upgrade npm to v6.5.0 by ljharb · Pull Request #25234 · nodejs/node · GitHub

/ node Public

deps: upgrade npm to v6.5.0 - #25234

Closed
ljharb wants to merge 1 commit into
nodejs:masterfrom
ljharb:npm-6.5.0
Closed

deps: upgrade npm to v6.5.0#25234
ljharb wants to merge 1 commit into
nodejs:masterfrom
ljharb:npm-6.5.0

Conversation

ljharb commented Dec 26, 2018
edited
Loading

Copy link
Copy Markdown
Member

I noticed that node v11.6 ships with an npm that reports a version of v6.5.0-next.0. This PR updates that to actual v6.5.0.

Checklist

nodejs-github-bot added the npm Issues and PRs related to the npm client dependency or the npm registry. label Dec 26, 2018

ljharb commented Dec 26, 2018

Copy link
Copy Markdown
Member Author

cc @MylesBorins @zkat @iarna

ljharb commented Dec 26, 2018

Copy link
Copy Markdown
Member Author

Local test failures:

        not ok 9 - config properties list: projectData: # of elements
          ---
          found: 2
          wanted: 0
          compare: ===
          at:
            line: 66
            column: 5
            file: test/tap/config-basic.js
            type: global
            function: isDeeplyDetails
          stack: |
            isDeeplyDetails (test/tap/config-basic.js:66:5)
            test/tap/config-basic.js:77:7
            Array.forEach (<anonymous>)
            test/tap/config-basic.js:76:17
            lib/config/core.js:83:7
            Array.forEach (<anonymous>)
            lib/config/core.js:82:13
            f (node_modules/once/once.js:25:25)
            finalize (lib/config/core.js:193:5)
            afterExtras (lib/config/core.js:184:5)
            node_modules/mkdirp/index.js:48:26
          source: |
            t.is(Object.keys(aa).length, Object.keys(bb).length, msg + ': # of elements')
          ...

        not ok 51 - config by source -> project -> data: # of elements
          ---
          found: 2
          wanted: 0
          compare: ===
          at:
            line: 66
            column: 5
            file: test/tap/config-basic.js
            type: global
            function: isDeeplyDetails
          stack: |
            isDeeplyDetails (test/tap/config-basic.js:66:5)
            test/tap/config-basic.js:68:5
            Array.forEach (<anonymous>)
            isDeeplyDetails (test/tap/config-basic.js:67:19)
            test/tap/config-basic.js:68:5
            Array.forEach (<anonymous>)
            isDeeplyDetails (test/tap/config-basic.js:67:19)
            test/tap/config-basic.js:79:5
            lib/config/core.js:83:7
            Array.forEach (<anonymous>)
            lib/config/core.js:82:13
            f (node_modules/once/once.js:25:25)
            finalize (lib/config/core.js:193:5)
            afterExtras (lib/config/core.js:184:5)
            node_modules/mkdirp/index.js:48:26
          source: |
            t.is(Object.keys(aa).length, Object.keys(bb).length, msg + ': # of elements')
          ...

Copy link
Copy Markdown
Contributor

@ljharb is that test failing on master as well?

ljharb commented Dec 27, 2018
edited
Loading

Copy link
Copy Markdown
Member Author

Hmm, I'm seeing many more of the npm tests failing on master - but maybe I'm not running the tests properly. Some help, or independent confirmation, would be appreciated.

Trott commented Dec 29, 2018

Copy link
Copy Markdown
Member

Definitely tests on master are failing. It appears to have been this way for years. I tried to bisect to find out when the last time make test-npm passed. It's possible I got it wrong because maybe it had multiple periods of being fixed amidst long stretches of being broken, but the earliest time I could find was 3 years ago! 😱

Going through failures now and seeing about fixing them, but it's going to be slow-going. For example: #22519 (comment)

If you're seeing fewer test failures with the newer npm, that's probably a sign that a bunch of things have been fixed. I'd call that success.

jasnell commented Dec 29, 2018

Copy link
Copy Markdown
Member

Might be about time to revisit the question of whether we really need npm in our source tree. Sure, we can still huddle it in builds, but having it in the repo likely no longer makes sense (if it ever did).

ljharb commented Dec 29, 2018

Copy link
Copy Markdown
Member Author

In that case, can this land and go out in an 11.6.1? :-D

Trott left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

RSLGTM

Trott commented Dec 30, 2018

Copy link
Copy Markdown
Member

CI: https://ci.nodejs.org/job/node-test-pull-request/19879/

Trott added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Dec 30, 2018

ljharb commented Dec 31, 2018

Copy link
Copy Markdown
Member Author

@Trott any idea why that one FreeBSD job failed?

Trott commented Dec 31, 2018

Copy link
Copy Markdown
Member

@Trott any idea why that one FreeBSD job failed?

Known problem with the test that makes it fail from time to time.

Resume Build CI: https://ci.nodejs.org/job/node-test-pull-request/19889/

Trott commented Dec 31, 2018
edited
Loading

Copy link
Copy Markdown
Member

Hmmm...same FreeBSD failure. That's not reassuring. But it seems very unlikely to actually be related. Let's try one more time: https://ci.nodejs.org/job/node-test-pull-request/19890/

ljharb commented Dec 31, 2018

Copy link
Copy Markdown
Member Author

Yay, looks like that passed

Trott commented Jan 3, 2019

Copy link
Copy Markdown
Member

This is now 7 days old, has one approval, and a green CI. It can land. However, I'd really like it if it had a second pair of eyes giving it a review. Anyone? @nodejs/npm @lance

lance left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Looks good to me. I had the same failures, plus an additional three due to the fact that I have git configured to sign my commits. Updating my config with commit.gpgsign=false for deps/npm seemed to resolve the additional three I was seeing.

lance commented Jan 3, 2019

Copy link
Copy Markdown
Member

Landed in aea5229

lance closed this Jan 3, 2019
lance pushed a commit that referenced this pull request Jan 3, 2019
PR-URL: #25234
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Lance Ball <lball@redhat.com>
ljharb deleted the npm-6.5.0 branch January 3, 2019 16:08
addaleax pushed a commit that referenced this pull request Jan 5, 2019
PR-URL: #25234
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Lance Ball <lball@redhat.com>
refack pushed a commit to refack/node that referenced this pull request Jan 14, 2019
PR-URL: nodejs#25234
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Lance Ball <lball@redhat.com>
BridgeAR mentioned this pull request Jan 16, 2019
BridgeAR pushed a commit to BridgeAR/node that referenced this pull request Jan 16, 2019
PR-URL: nodejs#25234
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Lance Ball <lball@redhat.com>
Trott added the notable-change PRs with changes that should be highlighted in changelogs. label Jan 17, 2019
BridgeAR added a commit to BridgeAR/node that referenced this pull request Jan 17, 2019
Notable Changes

* compression / zlib:
  * Added brotli support (Anna Henningsen and Zach Vacura)
    nodejs#24938
* console:
  * Added `inspectOptions` option (Ruben Bridgewater)
    nodejs#24978
* crypto:
  * Always accept private keys as public keys (Tobias Nießen)
    nodejs#25217
* deps:
  * Upgrade npm to v6.5.0 (Jordan Harband)
    nodejs#25234
* fs:
  * Use internalBinding('fs') internally instead of
    process.binding('fs') (Masashi Hirano)
    nodejs#22478
* http(s):
  * Support overriding http\\s.globalAgent (Roy Sommer)
    nodejs#25170
* util:
  * Inspect ArrayBuffers contents closely (Ruben Bridgewater)
    nodejs#25006
* worker:
  * Expose workers by default and remove `--experimental-worker` flag
    (Anna Henningsen) nodejs#25361

PR-URL: nodejs#25537
BridgeAR added a commit that referenced this pull request Jan 18, 2019
Notable Changes

* compression / zlib:
  * Added brotli support (Anna Henningsen and Zach Vacura)
    #24938
* console:
  * Added `inspectOptions` option (Ruben Bridgewater)
    #24978
* crypto:
  * Always accept private keys as public keys (Tobias Nießen)
    #25217
* deps:
  * Upgrade npm to v6.5.0 (Jordan Harband)
    #25234
* fs:
  * Use internalBinding('fs') internally instead of
    process.binding('fs') (Masashi Hirano)
    #22478
* http(s):
  * Support overriding http\\s.globalAgent (Roy Sommer)
    #25170
* util:
  * Inspect ArrayBuffers contents closely (Ruben Bridgewater)
    #25006
* worker:
  * Expose workers by default and remove `--experimental-worker` flag
    (Anna Henningsen) #25361

PR-URL: #25537
MylesBorins mentioned this pull request Jan 24, 2019
targos mentioned this pull request May 7, 2019
BethGriggs pushed a commit that referenced this pull request May 10, 2019
PR-URL: #25234
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Lance Ball <lball@redhat.com>
MylesBorins pushed a commit that referenced this pull request May 16, 2019
PR-URL: #25234
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Lance Ball <lball@redhat.com>
ljharb mentioned this pull request Oct 29, 2019
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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 outstanding review comments, 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.

6 participants


Back | FazBrowse Home | New Git URL