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

Upgrade to Scala 2.13.10 #25717

Merged
merged 1 commit into from Nov 28, 2022
Merged

Upgrade to Scala 2.13.10 #25717

merged 1 commit into from Nov 28, 2022

Conversation

ioannakok
Copy link
Contributor

@ioannakok ioannakok commented Nov 25, 2022

Closes #25548

What does this change?

Tested

  • Locally
  • On CODE (optional)

Also changing sbt compiler option -target to -release as the former got deprecated with scala/scala#9982
@@ -22,7 +22,7 @@ object ProjectSettings {
scalacOptions ++= Seq(
"-unchecked",
"-deprecation",
"-target:jvm-1.8",
"-release:11",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

-target got deprecated in Scala 2.13.9.

I am not entirely sure why we had jvm-1.8 here. The minimum JVM version this code requires is 11 and -release:8 throws errors. I might be misunderstanding the difference between the old -target and new -release though. If anyone knows more about this, an explanation would be much appreciated since I would like to understand this better!

Copy link
Member

Choose a reason for hiding this comment

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

I am not entirely sure why we had jvm-1.8 here.

I think it was just missed in the various stages of adopting Java 11 (eg #24064 maybe).

Well done on using the colon in "-release:11" by the way! The PR description in scala/scala#9982 seems to suggest it should be "-release 11", but for that the scalac compiler says:

[warn] bad option '-release 11' was ignored

Choose a reason for hiding this comment

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

either form is ok, but two tokens in the build file, Seq("--release", "11"). (I prefer the colon.)

Worth adding that --target affects the "classfile version" but scalac generally does not leverage JVM features where it matters. TIL a decade ago the setting mattered more to scalac.

@ioannakok ioannakok marked this pull request as ready for review November 25, 2022 17:20
@ioannakok ioannakok requested a review from a team as a code owner November 25, 2022 17:20
@ioannakok
Copy link
Contributor Author

ioannakok commented Nov 25, 2022

Deployed successfully to CODE

Copy link
Member

@rtyley rtyley left a comment

Choose a reason for hiding this comment

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

Brilliant! Nice work.

@@ -22,7 +22,7 @@ object ProjectSettings {
scalacOptions ++= Seq(
"-unchecked",
"-deprecation",
"-target:jvm-1.8",
"-release:11",
Copy link
Member

Choose a reason for hiding this comment

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

I am not entirely sure why we had jvm-1.8 here.

I think it was just missed in the various stages of adopting Java 11 (eg #24064 maybe).

Well done on using the colon in "-release:11" by the way! The PR description in scala/scala#9982 seems to suggest it should be "-release 11", but for that the scalac compiler says:

[warn] bad option '-release 11' was ignored

@ioannakok ioannakok merged commit 33605c2 into main Nov 28, 2022
@ioannakok ioannakok deleted the ioanna/upgrade-to-scala-2.13.10 branch November 28, 2022 10:28
@prout-bot
Copy link
Collaborator

Seen on PROD (merged by @ioannakok 26 minutes and 42 seconds ago)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade frontend to Scala 2.13.10
4 participants