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 an environment variable to configure the npm path #2826

Merged
merged 3 commits into from
Oct 5, 2018

Conversation

jbeezley
Copy link
Contributor

@jbeezley jbeezley commented Oct 5, 2018

This replaces the old --npm flag that was omitted in the new build implementation.

@zachmullen
Copy link
Member

Why is this an envvar but other options are passed as args?

@jbeezley
Copy link
Contributor Author

jbeezley commented Oct 5, 2018

Because @brianhelba suggested it. I think it does make sense; this kind of thing is often done via env variables, but I don't have a strong opinion.

This replaces the old `--npm` flag that was omitted in the new build
implementation.
@zachmullen
Copy link
Member

Can we expose it as both?

girder/cli/build.py Outdated Show resolved Hide resolved
@giesberge
Copy link
Contributor

Can we add a try except with a bit more of a helpful error message? I totally missed the npm flag the first time I was playing with installing girder on windows.

@jbeezley
Copy link
Contributor Author

jbeezley commented Oct 5, 2018

I added a usage error that states

No npm executable was detected.  Please ensure the npm executable is in your
path, use the --npm flag, or set the "NPM_EXE" environment variable.

Does that error not trigger for you or are you saying the message is unclear?

@jbeezley jbeezley merged commit 3824dcd into master Oct 5, 2018
@jbeezley jbeezley deleted the npm-command-path branch October 5, 2018 18:52
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.

3 participants