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

Fix figgyPudding error in npm token #259

Closed
wants to merge 1 commit into from
Closed

Fix figgyPudding error in npm token #259

wants to merge 1 commit into from

Conversation

benblank
Copy link
Contributor

It seems that a couple of lines were missed when token.js was changed to use figgyPudding, which causes an error when trying to run that command in certain circumstances. This patch fixes that error.

Tested locally by applying the patch to npm 6.11.3.

It seems that a couple of lines were missed when token.js was [changed to use figgyPudding](4cf850d#diff-398ed3b014436a5204583323ea29320b), which [causes an error](https://npm.community/t/npm-token-err-figgypudding-options-cannot-be-modified-use-concat-instead/10288) when trying to run that command in certain circumstances.  This patch fixes that error.

Tested locally by applying the patch to npm 6.11.3.
@benblank benblank requested a review from a team as a code owner September 27, 2019 18:06
}
if (conf.otp) conf.auth.otp = conf.otp
if (conf.otp) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This second bit is actually not what you want. It means that npm token <whatever> --otp=123456 will throw away the auth.

Since we've already set auth to an object, we can modify that object, because it's not a key on the figgy pudding container.

I'll land this, but without that part, just posting this comment for posterity :)

@isaacs isaacs mentioned this pull request Oct 8, 2019
@isaacs isaacs closed this in bbc92fb Oct 8, 2019
@benblank benblank deleted the patch-1 branch February 24, 2020 20:02
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