-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
replace npm-registry-client with npm-registry-fetch #109
Merged
Conversation
This file contains 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
Credit: @zkat Reviewed-By: @jefflembeck
Oops: 7984206 did the wrong thing with the git params, and it breaks anyone trying to sign their git commits with npm version. Fixes: https://npm.community/t/1661 Credit: @zkat
Credit: @zkat
Fix browser opening under Windows Subsystem for Linux (WSL). Credit: @thijsputman
PR-URL: #70 Credit: @kemitchell Reviewed-By: @zkat
I found this page useful for setting up npm command completion. Thank you npm documentation team. This suggested changed is based on my system which runs Ubuntu where the completion directory path for bash is found in /etc/bash_completion.d and not /usr/local/etc/bash_completion.d I dug around docs for this in The Bash Manual https://www.gnu.org/software/bash/manual/bashref.html#Programmable-Completion and on some an old (sadly retired) Debian Admin Site https://debian-administration.org/article/317/An_introduction_to_bash_completion_part_2 which seemed supportive of the dir being in /etc but I do not have access to an OS X machine where I guess bash_completion.d could be in /usr/local/etc so I left it in. PR-URL: #72 Credit: @RobertKielty Reviewed-By: @zkat
See discussion here: https://npm.community/t/npm-dist-tag-add-with-2fa-enabled-fails-for-non-latest-tag-with-500/2432 TL;DR: > To be clear, you can specify an existing tag and it’ll change it. If you have 2FA enabled, you do need to specify `--otp` and if you don’t, you should get a `401` as above. Ref: https://npm.community/t/npm-dist-tag-add-with-2fa-enabled-fails-for-non-latest-tag-with-500/2432 PR-URL: #74 Credit: @scotttrinh Reviewed-By: @iarna Reviewed-By: @zkat
Fixes: https://npm.community/t/npm-ci-logs-success-to-stderr/2303 PR-URL: #79 Credit: @alopezsanchez Reviewed-By: @zkat
This fixes https://npm.community/t/unhelpful-error-message-when-publishing-without-logging-in-error-eperm-operation-not-permitted-unlink/1377/3 and the other dozen or so issues that that link references, and possibly many more involving poor error messages from errors thrown by the upload function. @zkat you mentioned you could take a look at any fixes / answer any questions, if you could look this over and let me know if this is a good / valid approach that would be fantastic, thanks! (it wasn't a race condition, luckily :P). it may also be helpful to add something like ``` if (!auth.token || !(auth.username && auth.password)) { log.warn('publish', 'not logged in') } ``` just before we even open the first file descriptor to make sure that even if the error message is completely wrong something in the log will give users a clue what may be going on. I took the method of looking for login creds from the logout method, I'm not sure that's valid or if alternatives to npm exist that don't require credentials but users could still publish to. Triage of the issue: 1. The upload function throws an error 2. As that error bubbles through [cacache](https://www.npmjs.com/package/cacache#with-tmp) it tries to delete the tmpdir as it should 3. It can't delete the temp dir as the upload function's readFileStream to the tar it was trying to upload is still open. 4. [cacache](https://www.npmjs.com/package/cacache#with-tmp) throws an error about it's inability to remove the dir, which suppresses the upload function's error. Fixes: https://npm.community/t/unhelpful-error-message-when-publishing-without-logging-in-error-eperm-operation-not-permitted-unlink/1377/3 PR-URL: #80 Credit: @macdja38 Reviewed-By: @zkat
* feat(cli, outdated): Adds 'Homepage' to outdated --long output. - `package.json`'s `homepage` property is displayed when using the `--long` option for `npm outdated` * test: npm outdated --long - Adds `homepage` to `--parseable` output. - Updates `npm outdated --long` test to include `homepage` in expected output. - Adds `homepage` to `npm-outdated` documentation. * fix: javascript standard style updates PR-URL: #81 Credit: @jbottigliero Reviewed-By: @zkat
Credit: @petkaantonov
Credit: @isaacs
Credit: @isaacs
Credit: @isaacs
Credit: @watson
zkat
force-pushed
the
zkat/registry-fetch-replacement
branch
from
December 5, 2018 21:23
6e607fe
to
813905c
Compare
aeschright
approved these changes
Dec 10, 2018
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
once more, with feeling