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 javaapi collection forwarders for 2.12 and 2.11 #551

Merged
merged 2 commits into from
Aug 26, 2022

Conversation

DieBauer
Copy link
Contributor

This PR adds 2.13 collection compatible forwarders for scala 2.11 and scala 2.12, see #346.

2.11 has converters in traits WrapAsJava and WrapAsScala, while 2.12 has them in AsJavaConverters and AsScalaConverters.
The method names are the same, but unfortunately both CollectionConverters objects have to be separately defined for both sources, because of this import.

Tests are added in a Java file, to verify that the methods can be called as java api.

@SethTisue
Copy link
Member

review by @scala/collections ?

@SethTisue
Copy link
Member

Well, this certainly looks plausible to me. Anyone else want to take a look? Otherwise I guess we'll optimistically merge.

@SethTisue
Copy link
Member

@DieBauer headerCreate should fix the headerCheck part of the sbt failure (it's from https://github.com/sbt/sbt-header), plus it looks like you need to run scalafmt on the new sources.

@DieBauer DieBauer force-pushed the collectionconverters-javaapi branch from 9442429 to 5c76f7b Compare August 10, 2022 18:36
@DieBauer
Copy link
Contributor Author

Thanks for the hints! headers added, and sources formatted according to scalafmt, and squashed in a single commit

@DieBauer
Copy link
Contributor Author

The 2.11 and 2.12 build fails the versionPolicyMimaCheck. @SethTisue I'm not sure what to do here?

[error] scala-collection-compat: Failed binary compatibility check against org.scala-lang.modules:scala-collection-compat_2.11:2.8.1! Found 2 potential problems
[error]  * class scala.jdk.javaapi.CollectionConverters does not have a correspondent in other version
[error]    filter with: ProblemFilters.exclude[MissingClassProblem]("scala.jdk.javaapi.CollectionConverters")
[error]  * object scala.jdk.javaapi.CollectionConverters does not have a correspondent in other version
[error]    filter with: ProblemFilters.exclude[MissingClassProblem]("scala.jdk.javaapi.CollectionConverters$")
[error] Module scala-collection-compat:2.8.1+3-64b881ac-SNAPSHOT is not source compatible with version 2.8.1. You have to relax your compatibility intention by changing the value of versionPolicyIntention.
...
[error] java.lang.RuntimeException: Failed binary compatibility check against org.scala-lang.modules:scala-collection-compat_2.11:2.8.1! Found 2 potential problems
[error] (compat211 / versionPolicyMimaCheck) java.lang.RuntimeException: Failed binary compatibility check against org.scala-lang.modules:scala-collection-compat_2.11:2.8.1! Found 2 potential problems

@julienrf
Copy link
Contributor

You need to change the versionPolicyIntention to Compatibility.BinaryCompatible.

@DieBauer
Copy link
Contributor Author

I see the policy was changed with this PR: #553
Can that be reverted in this PR, or as a new one to master?

@SethTisue
Copy link
Member

in this PR is fine, but in its own commit is best

Adds 2.13 collection compatible forwarders for scala 2.11 and scala 2.12, see scala#346.

2.11 has converters in traits WrapAsJava and WrapAsScala, while 2.12 has them in  AsJavaConverters and AsScalaConverters.
The method names are the same, but unfortunately both CollectionConverters objects have to be separately defined for both sources, because of this import.

Tests are added in a Java file, to verify that the methods can be called as java api.
@DieBauer DieBauer force-pushed the collectionconverters-javaapi branch from 5c76f7b to 39b57cf Compare August 15, 2022 08:51
@SethTisue SethTisue self-assigned this Aug 16, 2022
@SethTisue SethTisue merged commit 15e8adf into scala:main Aug 26, 2022
@SethTisue
Copy link
Member

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants