Resource.allocated #419
Resource.allocated #419
Conversation
Codecov Report
@@ Coverage Diff @@
## master #419 +/- ##
=========================================
- Coverage 87.52% 87.3% -0.22%
=========================================
Files 69 69
Lines 1820 1828 +8
Branches 189 189
=========================================
+ Hits 1593 1596 +3
- Misses 227 232 +5 |
| @@ -137,6 +137,70 @@ sealed abstract class Resource[F[_], A] { | |||
| */ | |||
| def flatMap[B](f: A => Resource[F, B]): Resource[F, B] = | |||
| Bind(this, f) | |||
|
|
|||
| /** | |||
| case (_, ExitCase.Completed) => | ||
| F.unit | ||
| case ((_, release), ec) => | ||
| release(ec) |
There was a problem hiding this comment.
At a cursory look, what jumps out at me is that you're calling release here, irregardless of the ExitCase. And I thought the purpose of allocate is to not call release, but to save it for later.
Am I missing something?
There was a problem hiding this comment.
it's a bracketCase, not bracket. I'm calling release if the outer F fails or is interrupted during the process of acquiring resources, but if it completes properly, I then return the finaliser.
The rationale is here https://github.com/typelevel/cats-effect/pull/419/files#diff-ca16b2d95f362b8bc9b6c405f858be0cR146
There was a problem hiding this comment.
Ah, sorry, this is in case it isn't Completed.
|
PR looks good to me. Is it done? |
|
Code wise, I think it is. I would like some more tests but writing them appears tricky, maybe we can merge and iterate after? |
|
OK 🙂 |
No description provided.