Finish migration from Scalaz to Cats #75
Conversation
- Also cleanup CatsHelpers and remove straggling ~> conversions
Codecov Report
@@ Coverage Diff @@
## master #75 +/- ##
==========================================
+ Coverage 57.91% 58.21% +0.29%
==========================================
Files 128 128
Lines 4279 4269 -10
Branches 104 104
==========================================
+ Hits 2478 2485 +7
+ Misses 1801 1784 -17
Continue to review full report at Codecov.
|
adelbertc
force-pushed
the
cats-onwards
branch
2 times, most recently
from
76e52b5
to
0948ae0
Compare
- Also due to checking cats.Order laws for Instant, bring in cats-laws which brings in a new version of ScalaCheck which then requires us to bring in a new version of ScalaTest for bincompat. Dependencies are fun..
adelbertc
changed the title
|
Did the usual basic test on K8s - released a job, verified it deployed and ran, released a new version, verified old job was GC'd and expiration times worked as expected. Ready to be reviewed! |
| @@ -192,8 +187,7 @@ object Datacenter { | |||
|
|
|||
| object Namespace { | |||
| implicit def namespaceOrder: Order[Namespace] = | |||
| Order[String].contramap[Namespace](_.datacenter) |+| | |||
| Order[String].contramap[Namespace](_.name.asString) | |||
| Order.whenEqual(Order.by(_.datacenter), Order.by(_.name.asString)) | |||
There was a problem hiding this comment.
I'm not familiar with whenEqual but assuming the sematics remain the same.
There was a problem hiding this comment.
Yep! Some discussion here: typelevel/cats#2087
| import scalaz._ | ||
|
|
||
| // Triskaidekaphobia | ||
| object BiggerApplies { |
There was a problem hiding this comment.
It was a sad day when I had to write this (or was that Ross, I can't remember), but glad it see it go. Assuming you are using mapN in places where this would have been applicable.
There was a problem hiding this comment.
Yessir, I always wondered if they bothered to go up to 13, why not just go up the rest of the way to 22?
| /** Copied and adapted from Scalaz's Tag implementation. | ||
| * https://github.com/scalaz/scalaz/blob/v7.1.17/core/src/main/scala/scalaz/package.scala | ||
| */ | ||
| private[this] type Tagged[A, T] = { type Tag = T; type Self = A; } |
There was a problem hiding this comment.
I'm curious why this doesn't exist in cats. Is it something that should be added?
There was a problem hiding this comment.
There's been a couple competing implementations and nobody has converged on something yet, I think at this point a lot of folks are waiting for SIP-35 opaque types to land.
|
@adelbertc where you able to test Anyways this LGTM, seems to be pretty mechanical. Also, thanks for taking this on. |
|
@kaiserpelagic Just jobs, unfortunately routing is currently not implemented in the K8s backend yet (SHHHH), but now that the Cats migration is done I want to move on to doing that pretty soon. @timperrett and I have talked a couple of times of how that might look, essentially we're going to co-opt the K8s Service and Labels mechanisms. |
NonEmptyListandValidation\/andDisjunctionTCatsHelpersOptionT==>>toSortedMapand migrateOrderOrderlaws had to bring incats-laws.. which requires bringing in a new version of ScalaCheck.. which requires bringing in a new version of ScalaTest (yay dependencies)KleisliRWSTInterpreterscalaz.{@@, Tag}to Catshttp4s-argonaut61to completely remove Scalaz on classpath