Deprecate CartesianBuilder, finish up #1487#1745
Conversation
| parallelExecution := false, | ||
| requiresDOM := false, | ||
| jsEnv := NodeJSEnv().value, | ||
| jsEnv := new org.scalajs.jsenv.nodejs.NodeJSEnv(), |
There was a problem hiding this comment.
old one is deprecated.
| -XX:-UseGCOverheadLimit | ||
| # effectively adds GC to Perm space | ||
| -XX:+CMSClassUnloadingEnabled | ||
| # must be enabled for CMSClassUnloadingEnabled to work |
There was a problem hiding this comment.
@DavidGregory084 pointed out that these comments causes problem on Windows.
| * 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.
I think we can keep the toValidatedNel (instead of .leftMap(NonEmptyList.of(_)))
There was a problem hiding this comment.
missed that during the merge. will fix.
|
|
||
| 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.
Maybe we can just write H.map2 instead of the syntax version?
peterneyens
left a comment
There was a problem hiding this comment.
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.