tokio::signal::unix::Signal and the Stream trait

[andrewbaxter]

Was there a regression of #1848 ?

https://docs.rs/tokio/latest/tokio/signal/unix/struct.Signal.html (1.25.0) repeatedly refers to it as a stream but there's no stream implementation which confused me for a great while.

[brodybits]

Was there a regression of #1848 ?

It looks to me like this was added in PR #1849 but with no tests and removed in 8efa620 (PR #3277).

[Darksonn]

Thank you for pointing this out. The documentation should include a link to tokio_stream::wrappers::SignalStream so that people can more easily discover how to get a Stream out of it. (It is intentional that it does not implement Stream anymore.)

I'm marking this as help-wanted. To fix this issue, please submit a PR that updates the documentation to include such a link. (See the docs for other stream-like types for an example way to phrase this.) If it is missing on the windows variants of Signal, then it should also be added there.

[brodybits]

Correct, and I have noticed a few more things:

• There seems to be a lack of tests & documentation for tokio_stream::wrappers::SignalStream. I was able to comment out all impl blocks from tokio-stream/src/wrappers/signal_unix.rs without triggering any test failures.
• The documentation for Signal "repeatedly refers to it as a stream" (emphasis is mine), and I think it should be reworked.

Unfortunately I am not super-familiar with this functionality yet and am still pretty new to Rust, so I cannot promise too much meaningful progress anytime soon.

I would probably recommend updating the title (again) and removing the "E-easy" label to improve this situation.

---

P.S. I think the same test & documentation updates are needed for Windows as well.

[Darksonn]

The reason that no tests were added to the SignalStream wrapper is that it is a completely trivial wrapper around Signal. (And also, tokio-stream isn't tested very thoroughly in general.) Regardless, I would like to keep this one focused on the documentation issues in the main Tokio crate. Feel free to open a different issue about tests for the wrapper.

[amab8901]

is the issue solved now that there is a merged PR?