cats icon indicating copy to clipboard operation
cats copied to clipboard

Published 20 hours ago • verified publisher icon typelevel

Issue #4631: Make Later covariant

Open yanns opened this issue 1 year ago • 1 comments

Same as https://github.com/typelevel/cats/pull/937

fixes https://github.com/typelevel/cats/issues/4631

yanns avatar Jul 04 '24 15:07 yanns

Thank you for the fix! Note that Later is one of the Leaf descendants. The other two are Always and Now. I wonder if it makes sense to make all the "leaves" covariant together rather than just one of them? I feel it could be rather confusing to have one Leaf covariant and the other two invariant.

satorg avatar Jul 04 '24 20:07 satorg

Thank you for the fix! Note that Later is one of the Leaf descendants. The other two are Always and Now. I wonder if it makes sense to make all the "leaves" covariant together rather than just one of them? I feel it could be rather confusing to have one Leaf covariant and the other two invariant.

I had the same thoughts when changing Later, but was not feeling confident enough to make more changes than the very strict necessary. Thanks for the quick feedback! I've now adapted all leafs of Eval.

yanns avatar Jul 05 '24 07:07 yanns

@yanns , I lost tracking of your PR, sorry. Would you mind updating it to the most recent "main" such that we could re-run all the checks please? Thank you!

satorg avatar Sep 01 '24 06:09 satorg

@satorg thanks for coming back! I've updated the branch. The tests on scala 2.12 with scala native are failing, without any good error message.

yanns avatar Sep 02 '24 07:09 yanns

now all tests have passed

yanns avatar Sep 02 '24 19:09 yanns

When does a PR get merged? I could find any process described in https://github.com/typelevel/cats/blob/main/CONTRIBUTING.md ? Do you wait for 2 reviews?

yanns avatar Sep 03 '24 08:09 yanns