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 inliner for Scala 2 #429

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

mdedetrich
Copy link
Contributor

@mdedetrich mdedetrich commented Sep 11, 2023

Adds the scala inliner/optimizer for Scala 2 versions. The inline options that were picked are the ones determined to be safe for libraries (which sbt-library-management is).

The reasons for adding this is because

  • The latest Scala 2 inliner/optimizer is considered to be stable after many years of use
  • Its "free" performance especially for older versions of JDK (i.e. JDK 8/JDK 11). I suspect that the majority of people still use sbt this way and in some cases are forced to because of how prevalent JDK 8 is (even though its unfortunate)

One thing to note about the inliner is can cause development hitches locally (i.e. with Scala's incremental compiler). The easiest way to get around this would be to put the inliner behind an argument flag so that CI turns on the inliner (both for PR checks and for publishing) but by default inliner is off so that it doesn't mess with local development. The reason I didn't do this is because it appears that publishing is still done manually on someones machine (I can't see a github action for publishing) and hence it would be easy to forget to enable the inliner when publishing a new version of this library.

I guess if the standard "push tag to trigger a CI" publishing is added to this repo I can then update the PR with this suggestion (I would actually make the inliner an `AutoPlugin).

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.

1 participant