Open wunderk1nd-e opened 3 months ago
Yeah, that's definitely a bug; either in the code or in the docs.
I'm reminded of a similar confusion with embedNever
which we discussed in https://github.com/typelevel/cats-effect/pull/3351#discussion_r1070651800.
yeah, tbh I wasn't sure which way I should go fix the docs or fix the code. My intuition is joinWithNever
seems safer as the doc says then the current implementation. I'm happy to fix the doc too if We want to keep the current semantic of joinWithNever
.
@lenguyenthanh your PR passes our test suite so that's a good sign that we are not depending on the bug and hopefully nobody else is either. Also we have precedent for fixing bugs even if they did cause some breakage ... (see the async_
bugfix in 3.5), not saying we want to repeat that necessarily, just saying we did do it 😅
I agree that We need to be careful about the changes. So, the question is what does it take to introduce this change? Or should we be safe and update the docs instead?
Hi there,
In the docs for the the
Spawn
typeclass at the very end it states:However, the
joinWithNever
implementation doesn't look like it actually makes any attempts at self cancellation and I think I've confirmed this with a small test app.which outputs the following to console