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

Add option to pass Auth header #23

Merged
merged 4 commits into from
Feb 11, 2018

Conversation

bytheway875
Copy link
Contributor

I was unable to add a private package because there was not a way to pass the auth token to the request, so I updated the cli to take an auth option and then updated the request function to send an authorization header.

There are also some strange formatting changes from the linter... it's breaking the lines in different places.

@@ -6,7 +6,7 @@
"scripts": {
"test": "jest",
"lint": "eslint src/ --fix",
"build": "babel src --out-dir lib --ignore **/*.test.js",
"build": "babel src --out-dir lib --ignore *.test.js",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The previous pattern was not matching because all of the test files are at the root of the src folder. Now the tests are not being built into lib 💃

packageName
} and its peerDeps were installed successfully.`;
let successMessage = `${
C.successText
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the linter can't make up it's mind regarding where it wants to do those line breaks...

`This command would have been run to install ${packageName}@${
version
}:`
`This command would have been run to install ${packageName}@${version}:`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Again, this is something that was picked up by the autolinting and changed.

@bytheway875
Copy link
Contributor Author

I also noticed that the test suite is failing, but it also seems to be failing on the same test in master and unrelated to the new code.

@nathanhleung
Copy link
Owner

Thank you for the pull and the comprehensive review comments. Also appreciate that you updated the README.

Will merge and release today.

@nathanhleung nathanhleung merged commit 3586331 into nathanhleung:master Feb 11, 2018
@nathanhleung
Copy link
Owner

nathanhleung commented Feb 11, 2018

Also, with regard to the tests, you're right - still working on them. Thanks for checking though.

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.

None yet

2 participants