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

refactor lib npm.js #1509

Closed
wants to merge 56 commits into from
Closed

Conversation

isaacs
Copy link
Contributor

@isaacs isaacs commented Jul 10, 2020

Cleaning up lib/npm.js, lib/help.js, bin/npm-cli.js, and creating 100% test coverage along the way.

Come and get it! Packed full of refactoring and shiny new unit tests.

@isaacs isaacs force-pushed the isaacs/refactor-lib-npm.js branch from bbabed3 to d71db29 Compare July 17, 2020 01:43
@isaacs isaacs requested a review from claudiahdz July 17, 2020 01:46
@isaacs isaacs marked this pull request as ready for review July 17, 2020 01:46
@isaacs isaacs requested a review from a team as a code owner July 17, 2020 01:46
@isaacs isaacs requested a review from ruyadorno July 17, 2020 01:47
@isaacs isaacs force-pushed the isaacs/refactor-lib-npm.js branch 2 times, most recently from 455721f to 0350ae5 Compare July 20, 2020 18:02
@isaacs isaacs mentioned this pull request Jul 20, 2020
@isaacs isaacs force-pushed the isaacs/refactor-lib-npm.js branch from 0350ae5 to 41f9a40 Compare July 20, 2020 23:13
@ruyadorno
Copy link
Contributor

@isaacs given the new default for npm ls I think it would be nice to also set all: true for either npm ll or npm la in order to make them more useful 😊

@isaacs
Copy link
Contributor Author

isaacs commented Jul 21, 2020

This PR fixes #1506 and #1533.

isaacs added 19 commits July 23, 2020 10:15
Also, modernize the code in the unsupported version checking module.
Allows openUrl to open `file://` urls, and passes that instead of the
raw path.
We don't use that any more, Arborist handles it all for us.
- Several sections of code moved out into separate modules.
- npm is an instance of an inline class.
- uses a Proxy for npm.commands object instead of the old hand-rolled
  Proxy-esque thing.
- organized code path for outputting usage/help information.
This removes a lot of very outdated dependencies, updates many to
their modern (usually promisified) versions, and updates (or removes)
code to account for the change.

Several dependencies have been completely removed, and others a bit
shuffled around, so that the node_modules folder can be bundled somewhat
more optimally than it would have otherwise.
@isaacs isaacs force-pushed the isaacs/refactor-lib-npm.js branch from 732c0cd to c97a878 Compare July 23, 2020 20:30
@isaacs isaacs force-pushed the isaacs/refactor-lib-npm.js branch from b371225 to 96d0f25 Compare July 24, 2020 21:34
Copy link
Contributor

@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 👍 :shipit:

@isaacs isaacs force-pushed the isaacs/refactor-lib-npm.js branch from e69258b to 4905fa8 Compare July 28, 2020 22:07
This resets the node_modules tree state after fixing the bug in
minipass-pipeline that caused some tarballs to be incompletely unpacked.
@isaacs isaacs force-pushed the isaacs/refactor-lib-npm.js branch from b7fd350 to d6755f0 Compare July 28, 2020 23:11
@isaacs isaacs force-pushed the isaacs/refactor-lib-npm.js branch 5 times, most recently from b518fae to a576bca Compare July 29, 2020 18:38
@isaacs isaacs force-pushed the isaacs/refactor-lib-npm.js branch from a576bca to ed5c7dc Compare July 29, 2020 18:42
isaacs added a commit that referenced this pull request Jul 29, 2020
PR-URL: #1509
Credit: @isaacs
Close: #1509
Reviewed-by: @ruyadorno
@isaacs isaacs closed this Jul 29, 2020
@isaacs isaacs deleted the isaacs/refactor-lib-npm.js branch October 2, 2020 21:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants