Add feature to retry process killing after specified duration

[zokker13]

Hi,

this PR fixes #105.
I'm wondering if the approach is good. If so, I'll add tests.

Personally, I'm not sure about the options parameter. It seems odd to be forced pass a parameter for the timeout but at the same time it's fine since there's a default value in the code.

[ehmicky]

Thanks for this PR @zokker13.

Sorry for the verbosity of the review!

Also at the end, don't forget to:

• add tests.
• make sure CI is passing.
• update the documentation.

Thanks!

[zokker13]

@ehmicky Thanks or the review. I implemented some changes, feel free to check them out again.
The CI seems to hate me - sometimes it works, sometimes not. I'll add docs/tests if you like the changes and deem them worthy.

[ehmicky]

Looks good to me! We need a review from @sindresorhus as well.

I added two minor suggestions. It does not look like CI has failed, so all good there :)

Thanks for this PR again! 🎉 🎉 🎉

[zokker13]

@ehmicky Implemented some more changes and added documentation. I have problems with the tests though. Maybe it's too early for me. Could have take a look if you have time?
8d06a6c

[ehmicky]

@zokker13 thanks for this new batches of updates! Hopefully we can merge this soon! 🎉 🎉

[zokker13]

@ehmicky Ready for another attempt ^^

[ehmicky]

Thanks! I think I figured out the problem with your test, see my comment. Also I added a suggestion for another test. And there are some linting issue with the fixture file. Thanks for holding on :)

[zokker13]

Thanks for finding that detail! I'm ready for anothr go :)

[ehmicky]

The tests are failing on CI for Windows.

From looking at the commit history, it started failing on Windows after adding the ipc option and waitForProcessLaunch().

[zokker13]

I think the ipc change is just the bad messenger since the tests never truly reported what's really there. I'll make a quick cross check with normal stdout as trigger tomorrow evening.

[zokker13]

@ehmicky wow, I was finally able to rebase master in here so you may check it out again.
git rebase --preserve-merges is truly a blessing. <3

[ehmicky]

Rebase looks good :)

[zokker13]

@ehmicky what else is needed to have it merged?

[sindresorhus]

Looks good now :)

[ehmicky]

Thanks a lot @zokker13!

I hope the code review process was not too tedious for you!

[zokker13]

It was okay. I didn't expect so much back and forth but that means people care for the package/feature so it's a good thing :)

[ehmicky]

Yes execa is used directly or indirectly by 2 million GitHub repositories so changes need extra care. I hope this was not too tedious, especially having to rebase.

This feature is great, thanks a lot for the effort put into it!

[zokker13]

Ya, the usage is pretty overwhelming.
But rebasing is fun and good practice I think. Once you figure it out that is ;)