improve liftTo/rethrowT/raiseOrPure syntax to be more flexible #2984
Conversation
|
Thanks! |
Codecov Report
@@ Coverage Diff @@
## master #2984 +/- ##
=======================================
Coverage 93.83% 93.83%
=======================================
Files 369 369
Lines 7072 7072
Branches 195 195
=======================================
Hits 6636 6636
Misses 436 436
Continue to review full report at Codecov.
|
| def raiseOrPure[F[_]](implicit ev: ApplicativeError[F, A]): F[B] = | ||
| ev.fromEither(eab) | ||
| @deprecated("use liftTo instead", "2.0.0") | ||
| def raiseOrPure[F[_]](implicit F: ApplicativeError[F, _ >: A]): F[B] = |
There was a problem hiding this comment.
If we're deprecating this, why change the signature? Should we just leave this as is?
There was a problem hiding this comment.
That's a good question… as far as I could tell, there was no downside to changing it, so I thought I might as well, in case we end up not deprecating it or it comes in handy for someone. 🤷♂
As long as the deprecated version is there too, it might also improve discoverability? I stumbled across liftTo for this purpose because I asked about raiseOrPure in Gitter and @kubukoz said it sounded like liftTo with worse types. Maybe better types for raiseOrPure makes it easier to find, and then the deprecation warning points you to the canonically-named method? Idk.
There was a problem hiding this comment.
The downside is that there is a small chance it breaks source for some users. If we deprecate it , there is really no reason to improve as it only encourage users to use it right?
There was a problem hiding this comment.
It makes sense to be conservative. I reverted the signature change and left the deprecation in place.
I'm a little curious when this change could break source, though… do you happen to know any examples?
There was a problem hiding this comment.
@bpholt it's probably going to be some extreme edge cases but imagine where there are two ApplicativeError instances for F in scope, one of them is ApplicativeError[F, A]
, the other is ApplicativeError[F, AA] where A is a subtype of AA
There was a problem hiding this comment.
Again it's not really expected to break some users, that's why we are totally okay with the other changes.
| @@ -350,6 +350,12 @@ class EitherSuite extends CatsSuite { | |||
| } | |||
| } | |||
|
|
|||
| test("raiseOrPure syntax works with specialized errors") { | |||
This takes the changes in #2880 and extends them to a few more methods with similar usage.