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

Fix mima command to run more quickly #415

Merged
merged 4 commits into from
Aug 29, 2022
Merged

Fix mima command to run more quickly #415

merged 4 commits into from
Aug 29, 2022

Conversation

lewisjkl
Copy link
Contributor

@lewisjkl lewisjkl commented Aug 26, 2022

Adds a matrix-specific command to allow mima to run more specifically

@lewisjkl lewisjkl mentioned this pull request Aug 26, 2022
@Baccata
Copy link
Contributor

Baccata commented Aug 26, 2022

I'm actually not sure I want this enabled on JS and Native. MiMa is adding 4 minutes of computation on each matrix cell, and although I think it's important to have it enabled to help people who do not instinctively spot bincompat problems, I'm okay with the idea that running it on JVM only is probably good enough at this point in the life of the library, in particular when some contributors run a LOT of commits on each PR ...

I know Scala isn't the most "green" language, far from it, but I'd rather we tried to keep computation usage low-ish, relatively speaking

@armanbilge
Copy link
Contributor

4 minutes is kind of outrageous. From a brief glance at your logs I wonder if it's a misconfiguration: for example the MiMa check for 2.13+js seems to be compiling your entire project.

@Baccata
Copy link
Contributor

Baccata commented Aug 26, 2022

@armanbilge, you're right.

@lewisjkl, in case you're not aware, we create cell-specific command here which let us invoke specific things for the particular combo run by CI. I'd expect a MiMa command to appear in this list, and for the GHA setup to have run: sbt mimaReportBinaryIssuesIfRelevant_$BUILD_KEY in it.

@lewisjkl
Copy link
Contributor Author

Looks like I need to tweak it a bit still, but I've got to head out for the day. I'll wrap it up on Monday.

@daddykotex daddykotex marked this pull request as draft August 29, 2022 13:57
@lewisjkl
Copy link
Contributor Author

I disabled mima checks as per the convo above, happy to change that back if we want. I also updated so it uses a command from the desiredCommands list.

@lewisjkl lewisjkl marked this pull request as ready for review August 29, 2022 18:20
@lewisjkl lewisjkl changed the title support js and native in mima Fix mima command to run more quickly Aug 29, 2022
@Baccata Baccata merged commit fb3e171 into main Aug 29, 2022
@Baccata Baccata deleted the mima-js-native branch August 29, 2022 18:35
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.

4 participants