Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Don't crash npm test when hg/git are missing #5212

Merged
merged 1 commit into from
Oct 1, 2018
Merged

Don't crash npm test when hg/git are missing #5212

merged 1 commit into from
Oct 1, 2018

Conversation

gaearon
Copy link
Contributor

@gaearon gaearon commented Oct 1, 2018

Fixes #5210 with a clunky workaround.
I hope this more or less matches the Jest logic.

@@ -31,15 +31,36 @@ if (process.env.SKIP_PREFLIGHT_CHECK !== 'true') {
// @remove-on-eject-end

const jest = require('jest');
const execSync = require('child_process').execSync;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this use react-dev-utils/crossSpawn?

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 dunno. I copy pasted from init.js. That's what we use there. Should I worry?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We also use it in CRA itself for detecting yarn. I think it's fine.

@SimenB
Copy link
Contributor

SimenB commented Oct 1, 2018

I hope this more or less matches the Jest logic.

Jest uses jest-changed-files, but that's probably overkill in your case. IMO the approach you take here is fine

@gaearon gaearon merged commit b103376 into master Oct 1, 2018
Copy link
Contributor

@rogeliog rogeliog left a comment

Choose a reason for hiding this comment

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

@Timer Timer deleted the test-git branch October 1, 2018 22:06
zmitry pushed a commit to zmitry/create-react-app that referenced this pull request Oct 29, 2018
@lock lock bot locked and limited conversation to collaborators Jan 19, 2019
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants