-
-
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
Apply syntax for tuples #1487
Apply syntax for tuples #1487
Conversation
blocked by #1073 |
Now that we have enabled the partial unification fix (#1583) and dropped I would really like to see this in 1.0.0-MF. I think the sooner we add this, the better. I think it might be nice to keep the cartesian builder ( Do you still have time to work on this @DavidGregory084? Otherwise I am happy to pick this up. |
@peterneyens I am leaning slightly towards removing the cartesian builder (|@|) in 1.0.0-MF. Mainly because that 1.0.0-MF is a "proposal" for the final API in 1.0.0, it would be a bit weird to have a deprecated thing in the proposal. |
Makes sense. Just though that having a deprecation cycle before 1.0.0 might make things easier, especially for something like Paging @johnynek who raised some upgrade/adoption concerns earlier in #1635 (comment). |
b900413
to
9f8f898
Compare
-XX:+CMSClassUnloadingEnabled | ||
# must be enabled for CMSClassUnloadingEnabled to work | ||
-XX:+UseConcMarkSweepGC |
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.
Comments in here cause Could not find or load main class: #
when starting sbt on Windows
9f8f898
to
eefd70b
Compare
Codecov Report
@@ Coverage Diff @@
## master #1487 +/- ##
=======================================
Coverage 93.94% 93.94%
=======================================
Files 241 241
Lines 4096 4096
Branches 156 159 +3
=======================================
Hits 3848 3848
Misses 248 248
Continue to review full report at Codecov.
|
@peterneyens @kailuowang Sure, I have updated the PR. I restored Cartesian builders, let's see how the discussion about deprecating them goes once everyone who's interested has weighed in. |
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.
👍 from me.
If we keep |
@DavidGregory084 I am just curious if you have time to continue on this, I think we reached a consensus that we just need to mark CartesianBuilder stuff as deprecated and update the docs to use new tupple syntax. Sorry for dragging so long and now trying to push it with release date past due. I'll gladly complete the rest if you want. |
@kailuowang Ah sorry, I wasn't clear that we had reached a consensus. :) I should have some time to work on this, but probably not until the end of the week, so if you think you are going to have time earlier than that please feel free to take this on. |
@DavidGregory084 thanks so much! I was just checking if I can find something to work on. Will leave this one to you then. |
@DavidGregory084 still have the bandwidth to finish up this one? I'll be more than happy to help if you don't. |
Thanks @kailuowang, I'm struggling a bit to find the time so it would be great if you could 👍 |
no problem @DavidGregory084 , #1745 is submitted to finish up this one. Closing this for now. |
I missed where we reached consensus to drop CartesianBuilder. Can we link to that here? I think this is a bit unfortunate because I know of a few code bases using @ and this seems like a minor stylistic change that will put work on a lot of folks. Removing Xor kept us on an old version of cats for a while at Stripe because the activation energy to go remove it everywhere was pretty high. I'm afraid this could be similar (though smaller in scope). |
Hey @johnynek I was going through the PRs that introduced breaking changes in 1.0.0 and I stumbled upon your comment. Did you have a chance to try the Scalafix rewrites? They should solve (or at least mitigate greatly) that specific problem :) |
Move apply syntax for tuples under
cats.syntax.apply._
(previouslycats.syntax.tuple._
). Methods are now suffixed byN
rather than the tuple's arity: