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)

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
About having a different carrier, I feel it would be a bit tricky
It was able to strip out the dependency to scalaz in the core module precisely because I fixed the internal representation of Step[A] to Future[Either[Result, A]]
If we were to enable an arbitrary carrier for Step, we would again need monad transformers
Valentin Kasas
@vil1
What other carrier do you have in mind ? scalaz.Task ?
Valentin Kasas
@vil1
About the the package names, there is a problem with having our packages FQN end wit scalazor cats since it would not mix well with other "real" scalaz or cats imports
I'm thinking about having rather something like io.kanaka.play.scalaz.compat._ or io.kanaka.play.scalaz.mode._
David R. Bild
@drbild

Ending with scalaz and cats should be OK as long as they aren't at the same level as a wildcard import.

import io.kanaka.play._
import compat.scalaz._ // or FQed as import io.kanaka.play.compat.scalaz._

is OK.

import io.kanaka.play._
import scalaz._ // or FQed as import io.kanaka.play.scalaz._

is bad because the io.kanaka.play.scalaz masks the real scalaz.

For the carriers, I have one project with Task and one with Clump (http://getclump.io/)
David R. Bild
@drbild
withFilter on Step looks wrong. Won't that throw an exception if the predicate doesn't hold? Version 1.x treats InternalServerError as the zero for Result, returning that inside a Left.
David R. Bild
@drbild
Here's a variant of version 2 with Step[A] generalized to StepT[F[_], A]: https://github.com/drbild/play-monadic-actions/commits/generalize-base-monad
I didn't update the scalaz or cats modules, so only the tests in core will pass.
This uses Daniel Spiewak's shims to abstract over the scalaz and cats typeclasses.
I could flesh this out, if you want to further consider it. But if you're set against the additional complexity I won't spend the time.
Valentin Kasas
@vil1
well, that's interesting !
Valentin Kasas
@vil1
David R. Bild
@drbild
Sure thing.
David R. Bild
@drbild
If this approach goes anywhere, we might experiment with making F[_] a parameter on MonadicActions, rather than having each implicit conversion polymorphic on F. Doing so might reduce the chance of inference issues and yield better error messages if any do crop up.

Something like

trait BaseMonadicActions[F[_]] {
  implicit def fOptionToStepOps[A](fOption: F[Option[A]])(implicit F: shims.Monad[F]): StepTOps[F, A] = ???

  ...
}

and

trait MonadicActions extends BaseMonadicActions[Future] {
   implicit val futureMonad: shims.Monad[Future] = ???
}

.

Valentin Kasas
@vil1
hey, someone has been busy lately XD
Valentin Kasas
@vil1
I've rejected your last PR (#21) but just pushed the same mechanism using no symlink
(snapshots are published on central)
Valentin Kasas
@vil1
hmm, there seems to be a specs2 exception with that setup :/
[play-monadic-actions-scalaz_7.1] $ test
[error] 
[error] 
[error] This looks like a specs2 exception...
[error] Please report it with the preceding stacktrace at http://github.com/etorreborre/specs2/issues
[error]  
[error] Error: Total 1, Failed 0, Errors 1, Passed 0
[error] Error during tests:
[error]     io.kanaka.play.ScalazStepOpsSpec
[error] (scalaz71/test:test) sbt.TestsFailedException: Tests unsuccessful
[error] Total time: 1 s, completed 20 avr. 2016 16:22:38
this is not really helpful
David R. Bild
@drbild
Dropping to an earlier specs 2 version (3.1, I think) leads to the more useful:
[error] Uncaught exception when running io.kanaka.play.ScalazStepOpsSpec: java.lang.ClassCastException: scalaz.Free$Gosub cannot be cast to scala.Tuple2
[trace] Stack trace suppressed: run last scalaz72/test:test for the full output.
[error] Error: Total 1, Failed 0, Errors 1, Passed 0
[error] Error during tests:
[error]         io.kanaka.play.ScalazStepOpsSpec
[error] 
[error] java.lang.ClassNotFoundException: io.kanaka.play.ScalazStepOpsSpec
[error] 
[error] STACKTRACE
[error]   java.net.URLClassLoader.findClass(URLClassLoader.java:381)
...elided
[error]   java.lang.Thread.run(Thread.java:745)
[error] 
[error] 
[error] This looks like a specs2 exception...
[error] Please report it with the preceding stacktrace at http://github.com/etorreborre/specs2/issues
[error]  
[error] Error: Total 1, Failed 0, Errors 1, Passed 0
[error] Error during tests:
[error]         io.kanaka.play.ScalazStepOpsSpec
Looks like it's still a scalaz compatibility issue
David R. Bild
@drbild
Oh, target needs to be an absolute path. file("scalaz71/target") is relative.
David R. Bild
@drbild
Submitted a PR for this, but you might have a cleaner way of fixing it that it doesn't require so much manual path wrangling.
David R. Bild
@drbild
Generalizing the base monad with my initial approach leads to ambiguous implicit issues. The alternative I mentioned above (BaseMonadicActions[F[_]] and MonadicActions extends BaseMonadicActions[Future] may be the way forward. That's the approach I've used successfully in the past.
First though, a different concern. Right now the implicit toStepOps defs in the compat modules are brought into scope via imports but those in core are brought in by extending the MonadicActions trait. It'd be nice to standardize.
David R. Bild
@drbild
Assuming we'll be generalizing the base monad at some point, perhaps something like this would allow both:
trait  BaseMonadicActions[F[_]] {
   implicit def optionToStepOps[A](option: Option[A]): StepTOps[F, A, Unit] = ???
   ...
}

trait ScalazMonadicActions[F[_]] {
   implicit def disjunctionToStepOps[A, B](disjunction: A \/ B): StepTOps[F, B, A] = ???
   ...
}

trait MonadicActions extends BaseMonadicActions[Future] // backward compatibility

object io.kanaka.play.future extends BaseMonadicActions[Future]
object io.kanaka.play.compat.scalaz.future extends ScalazMonadicActions[Future]

class ControllerViaTraits() extends Controller with BaseMonadicActions[Future] with ScalazMonadicActions[Future] {
   ...
}

class ControllerViaImports() extends Controller {
   import io.kanaka.play.future._
   import io.kanaka.play.compat.scalaz.future._

   ...
}
Any initial thoughts here? I can flesh this out into a PR (without the base monad generalization initially) if you want to see more.