Conversation
|
Thanks for the PR! I will review the changes this week. |
|
|
||
| import cats.Applicative | ||
| import cats.Functor | ||
| import cats.implicits.toFunctorOps |
There was a problem hiding this comment.
Makes things slightly more obvious:
| import cats.implicits.toFunctorOps | |
| import cats.syntax.functor._ |
We can apply the same change in all places.
|
Looks good overall, thanks! @NthPortal would you like to take a look? |
There was a problem hiding this comment.
this feature has a fundamental problem, which is that KindTransformers (perhaps better named LiftKind or KindLifter) do not (with the exception of the identity case) work in reverse.
as an example, one of the most common types for G is OptionT[F, *] (i.e. a wrapper around F[Option[*]]); you cannot unlift OptionT[F, *] back to F, because F[None] has no value to unwrap/unlift
| mct: MonadCancelThrow[F], | ||
| kt: KindTransformer[F, G], | ||
| tk: KindTransformer[G, F] |
There was a problem hiding this comment.
just some name bikeshedding:
- monadic constraints tend to just share the name of the HKT (idk why, but it's conventional)
ktis just the first letters of 'Kind' and 'Transformer', sotkdoesn't make a lot of sense as a name
| mct: MonadCancelThrow[F], | |
| kt: KindTransformer[F, G], | |
| tk: KindTransformer[G, F] | |
| F: MonadCancelThrow[F], | |
| ktFG: KindTransformer[F, G], | |
| ktGF: KindTransformer[G, F] |
| for { | ||
| state <- InMemoryMeterSharedState.create[IO](resource, scope, 0.seconds) | ||
| builder = SdkCounter.Builder[IO, Long](name, state.state, None, None) | ||
| mappedBuilder = builder.mapK[IO] |
There was a problem hiding this comment.
this is an identity transformation
|
|
||
| /** Modify the context `F` using an implicit [[KindTransformer]] from `F` to `G`. | ||
| */ | ||
| def mapK[G[_]: MonadCancelThrow](implicit |
There was a problem hiding this comment.
I think when both F ~> G and G ~> F are required, the method is more typically named imapK, e.g. in InvariantK from cats-tagless.
|
We can try implementing |
I've seen the implementation of mapK for Tracer and thought of doing the same for Meter.
I don't pretend that what I'm suggesting is a good implementation though. Also I didn't really get where is the best place to implement tests. So I did only an example one, for counter
Also, there is a point could be made that it is possible to make less amount of boilerplate code (in observable components at least) by encapsulating Builder trait
So, it's more of a feature suggestion
But if somebody will like the idea and will be able to tell me more about project structure then I'm ready to finish this pr