-
Notifications
You must be signed in to change notification settings - Fork 158
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 support for Scala 3 #428
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the pr @pikinier20!
So I think instead of having specific instructions for Scala 3 it'd be better to instead just update the code here:
sbt-scoverage/src/main/scala/scoverage/ScoverageSbtPlugin.scala
Lines 158 to 172 in 73f8f92
} else if ( | |
// TODO this is very temporary until support for this gets merged in. | |
// For now we restrict this to this exact SNAPSHOT version which needs | |
// to be published localled in order to test | |
coverageEnabled.value && scalaVersion.value == "3.1.2-RC1-bin-SNAPSHOT" | |
) { | |
Seq( | |
"-coverage", | |
s"${coverageDataDir.value.getAbsolutePath()}/scoverage-data" | |
) | |
} else { | |
Nil | |
} | |
} | |
) |
All the logic is already there to automate the setting up of the scalacOptions
so the user doesn't have to worry about them. Then the user no matter if they are using Scala 2 or Scala 3 the experience is the same, just use coverage
or coverageEnabled
and the plugin will take care of everything. Can we change this pr to update this code instead of having instructions for manual steps the user needs to take?
Also now that we are officially adding support for Scala 3 we should also add a test for this in https://github.com/scoverage/sbt-scoverage/tree/V2/src/sbt-test/scoverage.
You'll also want to update the pr to point towards the V2 branch instead of Main. After we get this merged in I'll probably go ahead and just release the V2 so users won't have to worry about using a milestone release to use it with Scala 3.
@TheElectronWill Perhaps you want to continue the work here? I mean, I can do it but my intention was only to move this documentation from Dotty :) |
0a5a396
to
d0976bf
Compare
Just as you proposed, instead of adding documentation, I added necessary scalacOption in plugin. Also, I've added two tests for scala 3. Probably we should wait for Scala 3.2.0-RC1 before merging it because it'll be first stable version which supports |
Sorry, I didn't have the time to add it last week. Kudos for doing it @pikinier20 ! |
related to: scala/scala3#15110