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

Attempt at fixing CI issue on windows #7385

Merged
merged 3 commits into from
Jul 18, 2019
Merged

Conversation

heyimalex
Copy link
Contributor

Does CI run tests right from the source? Let's find out!

@iansu
Copy link
Contributor

iansu commented Jul 18, 2019

It looks like this did the trick? cc @ianschmitz

@heyimalex
Copy link
Contributor Author

heyimalex commented Jul 18, 2019

I feel pretty confident that removing the kill call won't negatively affect anything. It's pretty much the very last thing done before the job completes, and at that point all orphaned processed get killed anyway. Maybe if we spawned verdaccio again after there would be a port conflict, but we don't.

We could set +e before and set -e after, but really that's just ignoring the failure on some platforms. We could track the pid of the spawned process and then use that to kill it, but that's adding complexity for little benefit. I think just removing it and dealing with it later if anything breaks is the best way forward.

tldr: merge away! I'm gonna rewrite the commit message right now.

@heyimalex heyimalex marked this pull request as ready for review July 18, 2019 20:43
@iansu
Copy link
Contributor

iansu commented Jul 18, 2019

Works for me. Thanks!

@iansu iansu added this to the 3.1 milestone Jul 18, 2019
@iansu iansu merged commit 45e0703 into facebook:master Jul 18, 2019
@lock lock bot locked and limited conversation to collaborators Jul 23, 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.

3 participants