-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Fixes #1070 #1071
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
Fixes #1070 #1071
Conversation
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.
I think a better approach is to add them as (undocumented) command line flags, e.g. --iojs_org_mirror
. node-gyp will automatically pick up the npm_config_-prefixed environment variable.
Also, can you add tests, keep lines <= 80 columns and write up a nice commit log?
By adding them as command line flags you mean relying on a dependency like command-line-args? And then reading the config either by environment variable and command-line (which include npm_config automatic inheritance)? I never worked in this config-to-cli inheritance before, but I can try with this library. Any other suggestion? About the tests and nice commit log, I can work on that (linking to issue is just a personal preference). About keeping lines <= 80 columns, did you consider writing an Thanks in advance! |
We use nopt and some homegrown logic. You can find the list of switches in lib/node-gyp.js, grep for |
There are existing style violations and I don't think it's worth the churn to fix them, I just try to keep new ones from sneaking in. :-) |
When switching to
In order to not to reproduce these violations again, could you give me the specifications? |
Reviewing the code a little further I think |
Fixes issue #1070 by adding support to npm config for mirror variables.