-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add redeem and redeemWith #3146
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3146 +/- ##
==========================================
- Coverage 93.28% 93.25% -0.03%
==========================================
Files 376 376
Lines 7323 7343 +20
Branches 195 186 -9
==========================================
+ Hits 6831 6848 +17
- Misses 492 495 +3
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks! :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Delighted to see this one arrive.
This PR includes @alexandru's first commit from #2237 but removes the
EitherT
enhancements and focuses on the part that fixes #2161 (newredeem
andredeemWith
methods).See typelevel/cats-effect#191 for additional discussion.
I still think we should try to get the
EitherT
improvements merged, butredeem
seems like a higher priority for 2.1.0. I've also reverted some other changes in the original commit in order to preserve binary compatibility.Note that I've moved
redeem
fromMonadError
toApplicativeError
and have added syntax and syntax tests.