Skip to content

sigh, rename back to scala-collection-compat (and re-enable MiMa) #356

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

Merged
merged 2 commits into from
Sep 4, 2020
Merged

sigh, rename back to scala-collection-compat (and re-enable MiMa) #356

merged 2 commits into from
Sep 4, 2020

Conversation

SethTisue
Copy link
Member

reverts #327

@SethTisue
Copy link
Member Author

SethTisue commented Sep 3, 2020

MiMa is failing, but only on 2.11?! I'm out of work time for today — I'll look into it tomorrow if nobody beats me to it.

@SethTisue SethTisue changed the title sigh, rename back to scala-collection-compat sigh, rename back to scala-collection-compat (and re-enable MiMa) Sep 4, 2020
@SethTisue
Copy link
Member Author

SethTisue commented Sep 4, 2020

The failure is due to the addition of toMapExtensionMethods in #341. lightbend-labs/mima#205 explains why it only fails on 2.11 (trait encoding difference).

I believe we can filter this out because the trait in question, PackageShared, is private[compat].

@SethTisue SethTisue merged commit 167833d into scala:master Sep 4, 2020
@SethTisue SethTisue deleted the unrename branch September 4, 2020 01:43
@SethTisue
Copy link
Member Author

#362 has MiMa followups

@NthPortal
Copy link
Contributor

@SethTisue I think that method should be moved directly into the package object (which duplicates it because there are separate files for 2.11 and 2.12, but there's not much to do about that). that's what we've been doing for other things

@SethTisue
Copy link
Member Author

@NthPortal oops, I was going to look at that before release, but it slipped my mind. is it too late now, because bincompat?

@NthPortal
Copy link
Contributor

I... don't know enough about bincompat to say what we should do

@SethTisue
Copy link
Member Author

I... don't know enough about bincompat to say what we should do

Before I think too hard about that, I'd like to understand why you are suggesting the method should be moved directly into the package object.

which duplicates it because there are separate files for 2.11 and 2.12, but there's not much to do about that

I'm confused — isn't avoiding such duplication exactly what PackageShared is for?

@NthPortal
Copy link
Contributor

NthPortal commented Sep 13, 2020

the problem is, adding methods to PackageShared isn't binary compatible, I believe. see #292 and #288 (comment). this seems to be an issue for 2.11 bincompat

@SethTisue
Copy link
Member Author

ah, thanks for surfacing that. @julienrf @lrytz was I wrong to add the MiMa exclusion here?

@julienrf
Copy link
Contributor

was I wrong to add the MiMa exclusion here?

Indeed, the correct solution is to duplicate the implicit conversion into the 2.11 and 2.12 source trees, as April said in #356 (comment).

@NthPortal
Copy link
Contributor

What is the correct thing to do now?

  • move the method to the package objects, draft a new release ASAP, and make a note that the release was bad and shouldn't be used?
  • leave it as is?
  • have the method in both places?

@SethTisue
Copy link
Member Author

What is the correct thing to do now?

Let's discuss on #367

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