Add Signal.waitUntil #2815
Add Signal.waitUntil #2815
Conversation
| * to change, or waiting for the value of the `Signal` of an | ||
| * ordered `Stream[IO, Int]` to be greater than a certain number. | ||
| */ | ||
| def waitUntil(p: A => Boolean): F[Unit] |
There was a problem hiding this comment.
Since Signal isn't sealed isn't adding this method a breaking change? :(
There was a problem hiding this comment.
I'll add it to SignalOps and that should fix it I think
There was a problem hiding this comment.
Does it need to go on SignalOps? You could just put that default implementation and the Concurrent constraint directly here.
There was a problem hiding this comment.
I could yeah, I guess I just went with how map is implemented. @mpilquist opinions?
There was a problem hiding this comment.
I found a clear justification for having it as an instance method: scaladoc friendliness. I think map has a similar problem, but then you'd have the question of whether removing SignalOps entirely constitutes source breakage or not, so I'll punt on map for now
There was a problem hiding this comment.
We can keep SignalOps deprecated, just remove the implicit modifier. So there's no source-breakage but the map on Signal will clearly take precedence.
There was a problem hiding this comment.
I'll leave that for a potential future PR
| * to change, or waiting for the value of the `Signal` of an | ||
| * ordered `Stream[IO, Int]` to be greater than a certain number. | ||
| */ | ||
| def waitUntil(p: A => Boolean)(implicit F: Concurrent[F]): F[Unit] = |
There was a problem hiding this comment.
Apart from the question of stability discussed in the comment, that mentions that the signal should be such that a "positive" answer can be observed for some minimal time, could there be a problem here for efficiency.
Some Signals can change very frequently. If it does, there could be no limit to the number of values that self.discrete would emit. If so, could the .compile.drain result in a fiber that could keep running and exhaust CPU? If so, perhaps it could help if this method took a delay parameter, a FiniteDuration, so as to limit the sampling frequency on this signal.
There was a problem hiding this comment.
mm I think I disagree, the same concern would apply to everything that uses a Signal (e.g. pauseWhen) , forcing this noncompositional sampling parameter everywhere, and potentially slowing reaction times for the vastly more common case where this isn't a concern. Furthermore, in practice this is only a problem when using continuous , not discrete: discrete cannot change any faster than the source, which means that if discrete is changing too fast for your CPU, the source is also changing too fast, and so you already have a problem, and you have to solve at the source (e.g. with debounce). continuous does have this problem because it changes (~infinitely) faster then the source, as do things like Stream.repeatEval(queue.tryTake).unNone.
No description provided.