-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Deprecate CartesianBuilder, finish up #1487 #1745
Deprecate CartesianBuilder, finish up #1487 #1745
Conversation
@@ -67,7 +67,7 @@ lazy val commonJsSettings = Seq( | |||
scalaJSStage in Global := FastOptStage, | |||
parallelExecution := false, | |||
requiresDOM := false, | |||
jsEnv := NodeJSEnv().value, | |||
jsEnv := new org.scalajs.jsenv.nodejs.NodeJSEnv(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
old one is deprecated.
-XX:+CMSClassUnloadingEnabled | ||
# must be enabled for CMSClassUnloadingEnabled to work |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DavidGregory084 pointed out that these comments causes problem on Windows.
@@ -175,7 +175,7 @@ final case class EitherT[F[_], A, B](value: F[Either[A, B]]) { | |||
* scala> val v1: Validated[NonEmptyList[Error], Int] = Validated.invalidNel("error 1") | |||
* scala> val v2: Validated[NonEmptyList[Error], Int] = Validated.invalidNel("error 2") | |||
* scala> val eithert: EitherT[Option, Error, Int] = EitherT.leftT[Option, Int]("error 3") | |||
* scala> eithert.withValidated { v3 => (v1 |@| v2 |@| v3.toValidatedNel).map { case (i, j, k) => i + j + k } } | |||
* scala> eithert.withValidated { v3 => (v1, v2, v3.leftMap(NonEmptyList.of(_))).mapN { case (i, j, k) => i + j + k } } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can keep the toValidatedNel
(instead of .leftMap(NonEmptyList.of(_))
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missed that during the merge. will fix.
@@ -160,7 +160,7 @@ private[data] sealed trait Tuple2KTraverse[F[_], G[_]] extends Traverse[λ[α => | |||
def G: Traverse[G] | |||
|
|||
override def traverse[H[_]: Applicative, A, B](fa: Tuple2K[F, G, A])(f: A => H[B]): H[Tuple2K[F, G, B]] = | |||
(F.traverse(fa.first)(f) |@| G.traverse(fa.second)(f)).map(Tuple2K(_, _)) | |||
(F.traverse(fa.first)(f), G.traverse(fa.second)(f)).mapN(Tuple2K(_, _)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can just write H.map2
instead of the syntax version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason we call it "cartesian tuple syntax", but put it under the cats.syntax.apply._
import?
@johnynek please allow me to reply you here since any further change is probably going to be through this PR. I think @non first suggested the possibility of deprecating the |
Codecov Report
@@ Coverage Diff @@
## master #1745 +/- ##
=========================================
- Coverage 94.02% 94% -0.03%
=========================================
Files 256 256
Lines 4201 4201
Branches 84 89 +5
=========================================
- Hits 3950 3949 -1
- Misses 251 252 +1
Continue to review full report at Codecov.
|
Okay. Thanks for linking to the relevant discussion. |
@peterneyens I think I addressed all your comments, have time to take another look? |
merge with 2 sign-offs given this has been discussed in depth in the previous related PRs. |
If |
@fosskers yes. I gave an example in the migration doc. Let me know if that works for you. https://github.com/kailuowang/cats/blob/8df835fe4700455135140c2e3d02b6555328985c/CHANGES.md |
This PR is to finish up #1487. I deprecated the
CartesianBuilder
, merged the master and one small correction in the build file.