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 node engine to package.json and call checkNodeVersion function in cli #88

Merged
merged 1 commit into from
Jul 23, 2016

Conversation

conorhastings
Copy link
Contributor

closes #82

cc @gaearon

@ghost ghost added the CLA Signed label Jul 22, 2016
@vjeux
Copy link
Contributor

vjeux commented Jul 22, 2016

Can you comment on how you tested this feature? Thanks!

@conorhastings
Copy link
Contributor Author

@vjeux sure:

  1. looked into how react-native cli uses their checkNodeVersion
  2. added the engine to the package and added the call to checkNodeVersion in the run of this cli
  3. used nvm to switch to node 0.12.7
  4. mved the code from my working branch of the code into the globally installed node version folder
  5. edited local code to insert the engine (not ideal, would love feedback on how to do it better)
    screen shot 2016-07-22 at 3 23 13 pm
  6. ran create-react-app my-app and observed error
    screen shot 2016-07-22 at 3 23 04 pm

@vjeux
Copy link
Contributor

vjeux commented Jul 22, 2016

Awesome test plan thanks!

It looks like it still lets you keep running the command anyway (it just prints a red warning), should we process.exit(1) there?

@ghost ghost added the CLA Signed label Jul 22, 2016
@conorhastings
Copy link
Contributor Author

@vjeux updated

@ghost ghost added the CLA Signed label Jul 23, 2016
@vjeux vjeux merged commit 10eae80 into facebook:master Jul 23, 2016
@vjeux
Copy link
Contributor

vjeux commented Jul 23, 2016

Thanks!

@gaearon gaearon added this to the 0.2.0 milestone Jul 25, 2016
@gaearon gaearon mentioned this pull request Jul 27, 2016
@lock lock bot locked and limited conversation to collaborators Jan 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't npm start new app, SyntaxError: Unexpected token => on a fresh install
3 participants