Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Repo info
Activity
  • Dec 30 2020 23:29
    dspasojevic opened #52
  • Oct 22 2020 19:56
    marko-asplund commented #44
  • Oct 22 2020 19:48
    marko-asplund commented #46
  • Oct 22 2020 10:59
    vil1 opened #51
  • Oct 22 2020 10:58
    vil1 edited #50
  • Oct 22 2020 10:58
    vil1 opened #50
  • Oct 22 2020 10:58
    vil1 labeled #50
  • Oct 22 2020 10:16
    vil1 commented #44
  • Oct 22 2020 10:15
    vil1 closed #46
  • Oct 22 2020 10:13

    vil1 on master

    Update README.md (compare)

  • Oct 21 2020 22:14
    vil1 commented #46
  • Oct 21 2020 21:52

    vil1 on v2.2.1

    (compare)

  • Oct 21 2020 21:25

    vil1 on master

    Fix cross-building definition a… (compare)

  • Oct 21 2020 21:25
    vil1 closed #49
  • Oct 21 2020 21:25
    vil1 opened #49
  • Oct 21 2020 21:25

    vil1 on cross-build

    Fix cross-building definition a… (compare)

  • Oct 21 2020 21:21

    vil1 on sbt-ci-release

    (compare)

  • Oct 21 2020 21:21

    vil1 on sbt-ci-release

    Setup sbt-ci-release Fix build.sbt compilation Fix tests compilation (warning) and 1 more (compare)

  • Oct 21 2020 21:21

    vil1 on sbt-ci-release

    (compare)

  • Oct 21 2020 21:17

    vil1 on sbt-ci-release

    Fix cross-building definition a… (compare)

David R. Bild
@drbild
Basically it is:
for {
  s <- Future(42)
  t <- stepToResult( Future(Some("foo")) ?| NotFound )
} yield Ok(t)
So any interleavings of Step[A]s and Future[A]s will work if both stepToResult and futureToStep implicits (and a ExecutionContext) are in scope. But not in the intended way.
The postfix -| is still a cleaner solution. (And it's explicit, which can be nice)
If people don't want postfix ops enabled, typing .-| isn't that bad.
David R. Bild
@drbild

Yuck, the postfix form doesn't work at all in for comprensions without an explicit . or parens:

import scala.language.postfixOps
for {
  s <- (Future(42)          -|)
  t <-  Future(Some("foo")) ?| NotFound
} yield Ok(t)

or

for {
  s <- Future(42)          .-|
  t <- Future(Some("foo"))  ?| NotFound
} yield Ok(t)
David R. Bild
@drbild
(Scratch that earlier stuff about interleaving stepToResult and futureToStep. It only worked incidentally in my case.)
I've submitted a PR with the -| approach.
Valentin Kasas
@vil1
ok I merged it
Now about the scalaz/cats compatibility thing, I was thinking : we could roll our own EitherT definition/implementation
that way, we would have a core module depending only on play, and two submodules scalaz and cats adding support for the specific types (resp. \/ and Xor)
Valentin Kasas
@vil1
what do you think ?
David R. Bild
@drbild
We'll definitely need three modules (or have essentially two independent projects). If I'm understanding you correctly, maybe instead of a custom EitherT, just roll our own disjunction transformer typeclass with instances for EitherT and XorT in their respective modules? It'd be nice for Step[A] to be EitherT[Future, Result, A] or XorT[Future, Result, A] depending on the import. Not CustomEitherT[Future, Result, A], where CustomEitherT.runis Future[Xor[Result, A]in the cats module and Future[Either[Result, A]in the scalaz module.
The MonadicActions trait should still refer to Step, but Step should be defined by the specific import. Ideally, the rest of the ActionDSLpackage is just in core, with some sort of abstraction (e.g., MTL-esque typeclasses) to hide the multiple implementations of Step.
Valentin Kasas
@vil1
Well, not exactly
Valentin Kasas
@vil1
What I have in mind is rather : we implement Step[A] which is a FilterMonadic (with map, flatMap and filter methods) and method/implicit classes to lift the standard scala types into a Step (there's only a handful of such sensible types, Future, Either, Option, etc...)
This will of course require a little extra work (we wouldn't use the monad transformer machinery) but this way, we would remove the adherence to scalaz
and in separate "add-on" modules, we provide methods/implicit class to lift scalaz and cats sensible types into a Step(resp \/ and Xor)
Valentin Kasas
@vil1
That's just a suggestion though, I have no idea how it would be done exactly
David R. Bild
@drbild
I see.
I like that in the current version I can transition between Step[A] and EitherT[Future, Result, A] seamlessly. I'd hate to lose that, but we could just provide to toStep and toEitherT (toXorT) and enrichment methods in the scalaz (cats) module. That makes switching almost as easy.
David R. Bild
@drbild
I also like the current type alias approach because it makes understanding Step trivial when new to the library (assuming you already know the common monads/transformers).
But a custom Step, as you're suggesting, may be direct enough in implementation (compared to the hackery needed to support both Step = Either and Step = XorT) that it might be easiest-to-understand approach moving forward.
Valentin Kasas
@vil1
and we could also provide relevant instances for scalaz.Monad[Step] and cats.data.Monad[Step]
Valentin Kasas
@vil1
I'll give it a try and see what it looks like
David R. Bild
@drbild
sounds good
Valentin Kasas
@vil1
well, this Kanaka-io/play-monadic-actions#16 thing bothers me a lot
it feels very sad to have two operators for passing different forms of B => Result
Valentin Kasas
@vil1
so, about the 2.0 stuff
I have a (pretty simple) implementation of Step with no reference to scalaz
It compiles fine (but there is some ExecutionContext boilerplate I'll probably get rid of later
Now I have to make the test pass again ^^
Valentin Kasas
@vil1
done, please take a look at Kanaka-io/play-monadic-actions@8432acf and tell me what you think
Valentin Kasas
@vil1
See also #17 and #18
David R. Bild
@drbild
Awesome; I'll take a look this evening.
Valentin Kasas
@vil1
Ok, so it seems that our Step monad is lawful
(well, unless the underlying Future fails)
Valentin Kasas
@vil1
So, I've pushed a resolution for #17 (scalaz compat) on branch https://github.com/Kanaka-io/play-monadic-actions/tree/2.0 and published a 2.0.0-SNAPSHOT on maven central
Valentin Kasas
@vil1
and now there's a resolution for #18
(and a fresher snapshot on central)
Valentin Kasas
@vil1
but we're not done yet
Valentin Kasas
@vil1
I'm pretty sure naming the relevant packages scalaz and cats was a mistake
and there's probably a way to have less ExecutionContext pollution in the code
And of course, we still have to address #16 in a least possible breaking way
Oh and we'll need to revamp the docs as well
David R. Bild
@drbild
That implementation looks good to me. The packages scalaz and cats should probably be compat.scalaz and compat.cats so they aren't imported by import io.kanaka.play._
Should the main package move from controllers.ActionDSL to io.kanaka.playas well? The controllers.ActionDSL.MonadicActions import has always felt dirty to me. I'd rather import io.kanaka.play.MonadicActions.
David R. Bild
@drbild
In my latest project, I need a carrier that is not Future. For prior projects with similar constraints, I've just forked/rewritten this lib. Are you interested in trying to abstract over the base monad in version 2,?
Valentin Kasas
@vil1
Hi, sorry I lagged a bit, I was at ScalarConf last week end
Valentin Kasas
@vil1
so about the package layout, I agree we can do better. Our users should only have to extend MonadicActions to use the DSL