Skip to content

Conversation

@oprypin
Copy link
Member

@oprypin oprypin commented Apr 5, 2020

Rename Process#kill(Signal) to Process#signal(Signal) that we can eventually implement on Windows (if needed) with a deprecation notice. The name kill is confusing as it can mean two things - send signal to process, or send signal kill (9) to process when argument omitted. But currently default behavior of kill method without providing signal argument is to send term signal to process (simulates posix behavior of kill command, but we need to remove POSIXisms of apis to become really portable)

Closes #8642. I'm re-creating the PR to be able to resolve outstanding comments.

This probably should be squash-merged, with the above message.

@RX14 RX14 added breaking-change May have breaking effect in some edge cases. Mostly negligible, but still noteworthy. kind:refactor topic:stdlib:system labels Apr 5, 2020
Copy link
Member

@straight-shoota straight-shoota left a comment

Choose a reason for hiding this comment

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

Thanks for picking this up 👍
I've just some minor wording suggestions, to be a bit more precise (and also align with the existing documentation).

@straight-shoota
Copy link
Member

@oprypin Why did you remove the example which included specs for terminate?

@oprypin
Copy link
Member Author

oprypin commented Apr 8, 2020

@straight-shoota , #8642 (comment)

Funnily enough, that spec is exactly the same as the one preceding it, as kill and terminate are the same thing for now

@straight-shoota
Copy link
Member

I see. We could still run #terminate in a spec, though. Even if it's not possible / very difficult to test it behaves correctly. But we should make sure it compiles and runs.

@oprypin
Copy link
Member Author

oprypin commented Apr 8, 2020

(make sure to squash when merging)

@RX14 RX14 added this to the 0.35.0 milestone Apr 9, 2020
@RX14 RX14 merged commit f69c54c into crystal-lang:master Apr 9, 2020
carlhoerberg pushed a commit to carlhoerberg/crystal that referenced this pull request Apr 29, 2020
Co-authored-by: Jan Zajic <[email protected]>
Co-authored-by: Jan Zajic <[email protected]>
Co-authored-by: Benoit de Chezelles <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking-change May have breaking effect in some edge cases. Mostly negligible, but still noteworthy. kind:refactor topic:stdlib:system

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants