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

Remove code duplication #1241

Closed

Conversation

EnoahNetzach
Copy link
Contributor

I just realized that with #1195 I duplicated some code. This PR just removes it.

packageName,
'package.json'
);
checkNodeVersion(packageJsonPath);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not even sure if this line is still useful.
Looks like unreachable code.

Copy link
Contributor

Choose a reason for hiding this comment

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

This lets us increase minimal version with time if react-scripts imposes this but user doesn't update the CLI itself.

}
}

checkNodeVersion(path.resolve(__dirname, 'package.json'));

var currentNodeVersion = process.versions.node
if (currentNodeVersion.split('.')[0] < 4) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need this check then?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This allows us to check the minimal supported version asap for the whole cra to work.
The problem was that it took ~10 minutes with node v0.10 to get to an halt with error, using a lot of resources meanwhile as it still installs all the dependencies, even thou it warns about the problem.

@gaearon
Copy link
Contributor

gaearon commented Jan 9, 2018

This doesn't merge cleanly and I forgot what this is about. If it's still relevant please resubmit :-)

@gaearon gaearon closed this Jan 9, 2018
@EnoahNetzach
Copy link
Contributor Author

I don't think this is still relevant 🤷‍♂️

@EnoahNetzach EnoahNetzach deleted the remove-code-cuplication branch January 9, 2018 16:45
@lock lock bot locked and limited conversation to collaborators Jan 20, 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.

4 participants