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

Use path to gulp executable for spawn #479

Merged
merged 2 commits into from
Sep 10, 2018
Merged

Use path to gulp executable for spawn #479

merged 2 commits into from
Sep 10, 2018

Conversation

mikeshawdev
Copy link
Contributor

Previously the spawn command just took gulp as an argument, which worked when running the app via npm start. However, running node start would fail because it couldn't find gulp. It would look globally, whereas npm start would look locally. Referencing the path within node_modules directly means both commands can be used to start and run the app.

Resolves #393.

@NickColley
Copy link
Contributor

Hey @mikeshawdev,

Sorry for the long time before getting a response, this work looks good.

Would you be willing to update your branch so we can merge this? It would require a CHANGELOG entry for this bug fix.

If not I can clean this up on your behalf and get it in.

Nick

@mikeshawdev
Copy link
Contributor Author

@NickColley all updated 👍

Copy link
Contributor

@NickColley NickColley left a comment

Choose a reason for hiding this comment

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

I've tested this by pulling your change locally by running:

  1. npm start
  2. node start.js

I will need another reviewer to check this pull request and give an approval before we can merge this.

Thanks for the help Mike :)

@colinrotherham
Copy link
Contributor

@mikeshawdev @NickColley You could try using npx gulp instead since npm@5.2.0.

It'll find the gulp binary automatically.

Mike Shaw added 2 commits September 10, 2018 10:53
Previously the spawn command just took `gulp` as an argument, which worked when running the app via `npm start`. However, running `node start` would fail because it couldn't find `gulp`. It would look globally, whereas `npm start` would look locally. Referencing the path within `node_modules` directly means both commands can be used to start and run the app.
Signed-off-by: Mike Shaw <mike.shaw@dwp.gsi.gov.uk>
@NickColley NickColley merged commit b4de7fb into alphagov:master Sep 10, 2018
@NickColley
Copy link
Contributor

Thanks @mikeshawdev :)

@kr8n3r kr8n3r mentioned this pull request Sep 10, 2018
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.

4 participants