Remove dependency on signal-hook-tokio#387
Conversation
|
@mxpv @jsturtevant PTAL, thanks! |
| use futures::stream::poll_fn; | ||
| poll_fn(move |cx| -> Poll<Option<i32>> { | ||
| ready!(sig.poll_recv(cx)); | ||
| Poll::Ready(Some(kind.as_raw_value())) |
There was a problem hiding this comment.
I'm not sure I understand this part. Why do we poll manually here?
There was a problem hiding this comment.
The object returned by signal is not a Stream. The poll_fn function constructs a Stream based on a polling function.
We need to create a Stream so that we can use the SelectAll stream "merge" all the signal streams.
I'll add this as a comment in the code.
There was a problem hiding this comment.
An alternative would be to use tokio-stream's wrappers for Signals and CtrlC: https://docs.rs/tokio-stream/latest/tokio_stream/wrappers/struct.SignalStream.html
I don't think it's worth adding a new dependency when poll_fn works fine, but it would be a clearer without having to add a comment.
Signed-off-by: Jorge Prendes <jorge.prendes@gmail.com>
c5cc38c to
55e25d4
Compare
|
Looks like there are some unrelated issues with the Ubuntu 20.04 runners.
Is there any specific reason to test on Ubuntu 20.04? |
The
signal-hook-tokiopackage is now integrated into tokio in thesignalsfeature.Moreover, the
signal-hook-tokiocrates is unix only (see vorner/signal-hook#100 (comment)), while the tokio impl also supports windows.This PR is part of the effort to add Windows support for the async feature.