Use CanFail and NeedsEnv For ZPure (#401) #406
Conversation
|
|
| @@ -91,13 +92,13 @@ sealed trait ZPure[-S1, +S2, -R, +E, +A] { self => | |||
| self andThen that | |||
|
|
|||
| /** | |||
| * Runs this computatation if the provided environment is a `Left` or else | |||
| * Runs this computation if the provided environment is a `Left` or else | |||
There was a problem hiding this comment.
Thanks for this PR @spoltier 👍
I have a question regarding the explicit use of CanFail.
Unfortunately I'm no ZPure expert, so we will need to rely on @adamgfraser 's expertise.
| @@ -187,7 +188,7 @@ object ZPureSpec extends DefaultRunnableSpec { | |||
| }, | |||
| testM("orElseSucceed (Success case)") { | |||
| check(genInt, genInt, genInt) { (s1, v, v1) => | |||
| val (_, a) = ZPure.succeed(v).orElseSucceed(v1).run(s1) | |||
| val (_, a) = ZPure.succeed(v).orElseSucceed(v1)(CanFail).run(s1) | |||
There was a problem hiding this comment.
I'm not sure we should specify this explicitly. On the opposite, shouldn't this test verify, that when actual users will use this in their code, they won't have to specify it either?
There was a problem hiding this comment.
The user shouldn't ever have to specify it. For testing in some cases we may need ignore the CanFail warnings. For example here I think the intent is to test that when you call orElseSucceed on a successful value you just get the original success back, which makes sense but would normally cause an error.
A slightly nicer way to do this is to do the following within the scope of your test:
implicit val canFail = CanFailThis will silence the warnings for that scope.
There was a problem hiding this comment.
@adamgfraser Ok, I'll switch to the implicit declaration
| * runs that computation if the provided environment is a `Right`, returning | ||
| * the result in an `Either`. | ||
| */ | ||
| final def +++[S0 <: S1, S3 >: S2, R1, B, E1 >: E]( | ||
| that: ZPure[S0, S3, R1, E1, B] | ||
| ): ZPure[S0, S3, Either[R, R1], E1, Either[A, B]] = | ||
| )(implicit evL: NeedsEnv[R], evR: NeedsEnv[R1]): ZPure[S0, S3, Either[R, R1], E1, Either[A, B]] = |
There was a problem hiding this comment.
Don't think this is correct. Even if the environment type is any this is well defined. Just if we provide Left with anything we get one value and if we provide Right with anything we get the other value.
| * runs that computation if the provided environment is a `Right`, unifying | ||
| * the result to a common supertype. | ||
| */ | ||
| final def |||[S0 <: S1, S3 >: S2, R1, B, E1 >: E, A1 >: A]( | ||
| that: ZPure[S0, S3, R1, E1, A1] | ||
| ): ZPure[S0, S3, Either[R, R1], E1, A1] = | ||
| )(implicit evL: NeedsEnv[R], evR: NeedsEnv[R1]): ZPure[S0, S3, Either[R, R1], E1, A1] = |
There was a problem hiding this comment.
Same comment as above regarding +++.
|
Can we do something about |
Looks related to cla-assistant/cla-assistant#124 - some CLA assistant panel accessible to github org members / admins ? |
|
So closing and re-opening helped... you can tell, that GitHub is now operated by Microsoft 😂 |
Let me know if this makes sense, I may have forgotten some method.
Happy to do another pass later on sunday / monday once more ZPure methods (from issue #375) are there.