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

Enable Mima for native build #517

Merged
merged 2 commits into from
May 13, 2021
Merged

Enable Mima for native build #517

merged 2 commits into from
May 13, 2021

Conversation

ashawley
Copy link
Member

There are native artifacts to compare, now.

@SethTisue
Copy link
Member

Is it actually meaningful to do this?

@ashawley
Copy link
Member Author

I suppose not. We don't have any Scala native specific code at the moment. If it seems unnecessary we can keep it disabled. That's fine with me.

@SethTisue
Copy link
Member

@dwijnand is there any case for doing this? like even if we had Scala Native specific code, MiMa only checks .class files for them, so perhaps checking those is some kind of proxy for some kind of bincompat? iirc Scala.js always generates .class files anyway (for some reason I've heard Seb give and then forgot)

@dwijnand
Copy link
Member

I don't think so. I don't see why it wouldn't be some kind of proxy for some false positives. I don't know enough about what Scala Native emits and how the dependencies link together to talk intelligently, but I don't see anything helpful being achieved with this change.

@ashawley
Copy link
Member Author

I always assumed Mima was unable to grok actual compatibility for non-Java targets (Scala.js and native). Having Mima at least check that something didn't unexpectedly go missing because of an innocuous build change still seems worthwhile. It's not uncommon for libraries to have to compile and ship different source directories for certain versions of Scala or code target. As I said, scala-xml doesn't have any of those bits at the moment for native, but it seems like a good safety to have.

@SethTisue
Copy link
Member

I'm okay with enabling it but I would suggest including a comment referencing this discussion.

@ashawley
Copy link
Member Author

Ok, will do.

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