Skip to content
This repository has been archived by the owner on Oct 21, 2023. It is now read-only.

Drop Scala 2.12 + add Scala 3 (but not use it) #176

Merged
merged 6 commits into from
Aug 25, 2021

Conversation

mkurz
Copy link
Member

@mkurz mkurz commented Aug 20, 2021

No description provided.

@@ -17,14 +17,14 @@ $ exec git branch -u origin/main
# However, that initial state does not contain the above created .git folder (created via git init) yet.
> reload

> release cross with-defaults
Copy link
Member Author

@mkurz mkurz Aug 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason why I removed cross here is because IMHO this is not a real life use case. This test here, as its name plugin-with-root tells us, tests a sbt plugin that activates the PlayRootProject plugin (see PlayRootProjectBase.scala above). As you can see above, the PlayRootProject did set crossScalaVersion to 2.12 until now. Now, I changed that to 2.13, which makes the scripted test here fail because it can not resolve .../org/scala-sbt/scripted-sbt_2.13/1.5.5/...
That is, as we know, because sbt plugins for sbt 1 need to be published with scala 2.12.
I had a look at the projects that still use interplay (#150) and which, from these projects, make use of the PlayRootProject plugin. That are

Now it turns out that all of these projects explicitly set crossScalaVersions anyway, overriding the value set by PlayRootProject:

Now... scalatestplus-play and slick are not sbt-plugins anyway but, in interplay terms, libraries and therefore set their cross versions to 2.12 and 2.13. Play just sets it to empty list for the root project.

The conclusion for me is that PlayRootProject was meant to be used for "libraries" rather than sbt plugins and you don't really want to enable PlayRootProject if your project is an sbt plugin, but if you do, you want to set cross version explicitly.

Copy link
Member

@marcospereira marcospereira left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor comments, but LGTM.

@@ -16,9 +16,9 @@ object PlayRootProjectBase extends AutoPlugin {
override def projectSettings = PlayNoPublishBase.projectSettings ++ Seq(
crossScalaVersions := {
if ((ThisBuild / playCrossBuildRootProject).?.value.exists(identity)) {
Seq(ScalaVersions.scala212, ScalaVersions.scala213)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this if-else entirely?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And maybe even remove playCrossBuildRootProject and let other projects set crossScalaVersions explicitly if they need to.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this if-else entirely?

Yes, I was thinking the same yesterday, but wasn't 100% sure. Actually since this is about root projects, which usually don't get published, I think its ok if we remove the default.

And maybe even remove playCrossBuildRootProject and let other projects set crossScalaVersions explicitly if they need to.

Done. I set crossScalaVersions in the scripted tests explicitely. The projects that I mentioned in this thread will not be affected, because like written they already set the cross version explicitely anyway.

@mkurz mkurz requested a review from marcospereira August 23, 2021 20:37
Copy link
Member

@marcospereira marcospereira left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@@ -19,7 +19,6 @@ object PlayBuildBase extends AutoPlugin {
val playBuildExtraTests = taskKey[Unit]("Run extra tests during the release")
val playBuildExtraPublish = taskKey[Unit]("Publish extract non aggregated projects during the release")
val playBuildRepoName = settingKey[String]("The name of the repository in the playframework GitHub organization")
val playCrossBuildRootProject = settingKey[Boolean]("Whether the root project should be cross built or not")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙌🏼

@mergify mergify bot merged commit ec299ad into playframework:master Aug 25, 2021
@mkurz mkurz deleted the drop_scala212 branch August 25, 2021 16:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants