feat: Watcher::filter#15
Conversation
|
Documentation for this PR has been generated and is available at: https://n0-computer.github.io/n0-watcher/pr/15/docs/n0_watcher/ Last updated: 2025-10-01T09:50:13Z |
| }; | ||
| if filtered != self.current { | ||
| self.current = filtered.clone(); | ||
| return Poll::Ready(Ok(filtered)); |
There was a problem hiding this comment.
This behaves a bit different than I would have expected. Filter returns an Option instead of just TheOriginalValue, and will trigger even if the value goes from None to Some and Some to None. I would have expected it to return a Watcher instead.
it is a bit weird for filter to change the type of the stream.
There was a problem hiding this comment.
The alternative is to only have filter on watchables that contain Option<T> which is what I had first
There was a problem hiding this comment.
Most of our watchers have the value wrapped in an option, right?
I am not sure if this is a good API, but I definitely don't think having a fn filter that changes the stream type is intuitive. Maybe it just needs a different name?
There was a problem hiding this comment.
If you were to use this on our watchers in iroh of type Option, you would get an Option<Option>, and initialized would return immediately, possibly returning a None?
I think having this defined only on Optino is less confusing in comparison.
2 participants
Closes #14