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

fix: restart on change for non-default signals (#1409) #1430

Merged
merged 1 commit into from
Nov 5, 2018

Conversation

aaronjensen
Copy link
Contributor

Fixes #1409

@stale
Copy link

stale bot commented Oct 17, 2018

This issue has been automatically marked as idle and stale because it hasn't had any recent activity. It will be automtically closed if no further activity occurs. If you think this is wrong, or the problem still persists, just pop a reply in the comments and @remy will (try!) to follow up.
Thank you for contributing <3

@stale stale bot added the stale no activity for 2 weeks label Oct 17, 2018
@aaronjensen
Copy link
Contributor Author

Not stale. Please merge.

@stale stale bot removed the stale no activity for 2 weeks label Oct 17, 2018
@remy
Copy link
Owner

remy commented Oct 25, 2018

Can't merge until it passes the tests I'm afraid.

@aaronjensen
Copy link
Contributor Author

No idea how I missed your comment nor how this change would have broken a test, but I'll take a look.

When nodemon kills the child app in preparation for a restart, the app may exit
with a non-zero status code, especially when using custom signals like SIGINT.
Previously, this would be detected as a clean exit rather than a restart, at
least on macOS.

There was an existing flag, `killedAfterChange` which causes the desired
behavior, but it was only set on Windows. It seems that setting it always leaked
between tests and caused problems, so we only set it if a restart is expected.

Fixes remy#1409
@aaronjensen
Copy link
Contributor Author

@remy thanks for pointing out the failures. Fixed. Also added a test for the original issue. Cheers!

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.

2 participants