Skip to content
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

Add implicit: genericOrderedCompanionToCBF #249

Merged
merged 1 commit into from
Jan 20, 2020

Conversation

fdietze
Copy link
Contributor

@fdietze fdietze commented Sep 11, 2019

fixes #247

I don't know If I'm doing things right here... Help is appreciated.

Please review the new implicit def
What needs to be written in the test?
What needs to be done regarding binary compatibility?

Copy link
Contributor

@julienrf julienrf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for working on this @fdietze.

Here is my review, let me know if you have any question!

@julienrf
Copy link
Contributor

What needs to be done regarding binary compatibility?

We only promise backward compatibility, so your changes seem to be fine. Otherwise we can just bump the version number.

@fdietze
Copy link
Contributor Author

fdietze commented Sep 12, 2019

What needs to be done regarding binary compatibility?

We only promise backward compatibility, so your changes seem to be fine. Otherwise we can just bump the version number.

The version is 2.1.3-SNAPSHOT. should I change it?

@julienrf
Copy link
Contributor

Thanks @fdietze can you please sign the Scala CLA? https://www.lightbend.com/contribute/cla/scala

@fdietze
Copy link
Contributor Author

fdietze commented Sep 13, 2019

Thanks @fdietze can you please sign the Scala CLA? https://www.lightbend.com/contribute/cla/scala

done.

@julienrf
Copy link
Contributor

It seems that we are hitting this issue on 2.11: https://github.com/jatcwang/binary-compatibility-guide#dont-adding-methods-with-default-implementation-to-traits-211-or-before

The version is 2.1.3-SNAPSHOT. should I change it?

Yes, I believe this PR should target a 3.0.0 version. What do others think?

@fdietze
Copy link
Contributor Author

fdietze commented Jan 10, 2020

This PR would hopefully enable scala.rx to cross-compile for 2.13 (lihaoyi/scala.rx#138).

How about the versioning?

@NthPortal
Copy link
Contributor

@fdietze you can fix the bincompat issue by removing the method from PackageShared and adding it to each of the 2.11 and 2.12 package objects. You can see this strategy employed in #288 and #289. If you'd like, I'm happy to do it for you and force-push to this branch.

@fdietze
Copy link
Contributor Author

fdietze commented Jan 18, 2020

It would be nice if you could do it. I'm on holidays right now.
Thank you!

@NthPortal NthPortal force-pushed the fix-mutable-priority-queue branch from 60d9433 to c26c5e9 Compare January 20, 2020 07:57
@NthPortal NthPortal changed the title WIP: Add implicit: genericOrderedCompanionToCBF Add implicit: genericOrderedCompanionToCBF Jan 20, 2020
@NthPortal NthPortal added the library Related to compat library code (not rewrite rules) label Jan 20, 2020
@julienrf julienrf merged commit 10563e7 into scala:master Jan 20, 2020
@julienrf
Copy link
Contributor

Thank you both @fdietze and @NthPortal!

@mathieuleclaire
Copy link

Did you plan a release soon ? This issue is key to cross compile scala.rx.
Thanks

@julienrf
Copy link
Contributor

julienrf commented Feb 4, 2020

You’re right, we should cut a release soon because several improvements have been recently merged.

@NthPortal, do you think you would have time to finish #295 soon or should we do a release now and another one after #295 is merged?

@fdietze fdietze deleted the fix-mutable-priority-queue branch February 4, 2020 15:23
@NthPortal
Copy link
Contributor

@julienrf I'll try to finish up #295 tonight

@NthPortal
Copy link
Contributor

NthPortal commented Feb 5, 2020

it would be nice to get LazyList into the next release as well, but I understand if that's not a priority

@julienrf
Copy link
Contributor

julienrf commented Feb 5, 2020

@NthPortal I won’t be able to help on LazyList before some time, but I agree that we should eventually get it, and thanks a lot for what you’ve done so far.

@SethTisue Do you see anything we should do before pushing the tag?

@SethTisue
Copy link
Member

@julienrf I see no reason not to go ahead and release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
library Related to compat library code (not rewrite rules)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

.to(mutable.PriorityQueue) not compiling on 2.12
5 participants