Add Lift{Value,Kind} instances for Resource
#4478
Conversation
|
for reasons I do not understand, the implicits for |
| libraryDependencies ++= Seq( | ||
| "org.typelevel" %%% "cats-core" % CatsVersion | ||
| "org.typelevel" %%% "cats-core" % CatsVersion, | ||
| "org.typelevel" %%% "cats-mtl" % CatsMtlVersion |
There was a problem hiding this comment.
This is a meaningful extension of the dependency surface area. It's not something I would rule out, since most people depend on core or std, but it's still not something to do lightly.
There was a problem hiding this comment.
I agree; however, I thought having a PR would be more compelling than just opening an issue for it
| implicit def catsEffectLiftValueForResource[F[_]]( | ||
| implicit F: Applicative[F]): LiftValue[F, Resource[F, *]] = | ||
| liftValueImpl(F) |
There was a problem hiding this comment.
These two implicits should be inverted. The Applicative constraint is strictly less restrictive than MonadCancel, and so it should be consulted second. This means that you put the MonadCancel-constrained implicit in the 0 priority and Applicative in 1.
There was a problem hiding this comment.
thanks! that makes sense now that you've said it, though I confess I don't understand why doing it the wrong way makes it not compile
| import org.scalacheck.{Arbitrary, Gen} | ||
| import munit.DisciplineSuite | ||
|
|
||
| class ResourceSuite extends DisciplineSuite { |
There was a problem hiding this comment.
Fwiw, there are already resource tests in core, though I don't object to having a few more in laws.
There was a problem hiding this comment.
I believe the issue is that these tests depend on testkit (for PureConc), which core's tests do not have access to
NthPortal
force-pushed
the
Resource-lifts/PR
branch
from
ca64e91 to
68636bf
Compare
|
test failure seems spurious |
|
could someone with permissions please rerun the failed and cancelled ci jobs? |
2 participants
No description provided.