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

makes npm targets work on Windows #13

Merged
merged 9 commits into from
Apr 22, 2016
Merged

makes npm targets work on Windows #13

merged 9 commits into from
Apr 22, 2016

Conversation

jbuffin
Copy link
Contributor

@jbuffin jbuffin commented Apr 18, 2016

  • environment variables with cross-env
  • & works differently on windows so use concurrently instead
  • rm -rf is Unix only so use rimraf library

@jbuffin jbuffin mentioned this pull request Apr 18, 2016
@jbuffin
Copy link
Contributor Author

jbuffin commented Apr 18, 2016

I am having a problem running the test script as well on windows. If I remove the single quotes around the target regex, it works like this:

cross-env NODE_ENV=test cross-env NODE_PATH=./app mocha --compilers js:babel-register -r app/spec/support/setup.mocha.js --recursive app/spec/**/*.test.js -w

I will submit a separate PR for that.

@@ -3,10 +3,10 @@
"version": "0.0.0",
"description": "Universal Redux Template",
"scripts": {
"start": "NODE_PATH=./app nodemon app/server & node webpack.server.js & gulp css:watch",
"test": "NODE_ENV=test NODE_PATH=./app mocha --compilers js:babel-register -r app/spec/support/setup.mocha.js --recursive 'app/spec/**/*.test.js' -w",
"start": "cross-env NODE_PATH=./app concurrent --kill-others \"nodemon app/server\" \"node webpack.server.js\" \"npm run css:watch\"",
Copy link
Owner

Choose a reason for hiding this comment

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

The CSS job doesn't run since there's no npm script named css:watch. It should be gulp css:watch

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch, I totally missed that one

@mz026
Copy link
Owner

mz026 commented Apr 19, 2016

@jbuffin Thanks a lot! I left some comments on the commits. Also, could you please update npm-shringwrap.json accordingly?

@jbuffin
Copy link
Contributor Author

jbuffin commented Apr 19, 2016

@mz026 is there anything else you need on this?

@mz026
Copy link
Owner

mz026 commented Apr 19, 2016

@jbuffin for the npm-shrinkwrap.json, could you please include devDepencencies in it? The postinstall, which builds code after deployment, needs the devDependencies to build stuff.

@mz026
Copy link
Owner

mz026 commented Apr 20, 2016

@jbuffin any update on this?

@jbuffin
Copy link
Contributor Author

jbuffin commented Apr 20, 2016

Yes sorry, I got busy yesterday and didn't get a chance to rerun shrinkwrap with --dev. I will get that sometime today and push it.

@mz026
Copy link
Owner

mz026 commented Apr 20, 2016

no problem! thanks a lot! 👍

@jbuffin
Copy link
Contributor Author

jbuffin commented Apr 21, 2016

@mz026 I had to update one of the packages in order to get shrinkwrap to run correctly. All should be good now.

@mz026 mz026 merged commit 8bec6ef into mz026:master Apr 22, 2016
@mz026
Copy link
Owner

mz026 commented Apr 22, 2016

@jbuffin thanks!!

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