-
Notifications
You must be signed in to change notification settings - Fork 2
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
Ld/upgrade to java 21 #109
Conversation
@@ -67,7 +67,6 @@ val commonSettings: immutable.Seq[Def.Setting[_]] = List( | |||
"-deprecation", | |||
"-encoding", | |||
"UTF-8", |
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.
I removed this because I was getting an error that -release:21
is not a valid compiler option when I ran the code. Looking at the docs: https://docs.scala-lang.org/overviews/compiler-options/ my understanding is that this is not supported for Java 21.
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.
I tried on my local machine and it compiled successfully with -release:21
option. Did I miss anything here? (my local Java is 21.0.2-amzn
)
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.
Ah my mistake, yes seems to be working fine for me too now. Perhaps I had been running against an earlier version of Java when I tested it before
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.
I also got this error initially and can confirm it went away once I switched to java 21
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.
Yeah, the docs at https://docs.scala-lang.org/overviews/compiler-options/ are unhelpful there - I've opened scala/docs.scala-lang#3009 to try to improve them!
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.
Yeah, the docs at https://docs.scala-lang.org/overviews/compiler-options/ are unhelpful there - I've opened scala/docs.scala-lang#3009 to try to improve them!
💖
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.
Tested with java 21 locally and looks good to me 👍
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.
Awesome stuff - I haven't tested it out but very good to see updates to Java 21! ✨
@@ -16,7 +16,7 @@ jobs: | |||
- name: Set up JDK | |||
uses: actions/setup-java@v3 |
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.
It's not really part of this PR, but just spotted we can make this match ci-mobile-save-for-later.yaml
:
uses: actions/setup-java@v3 | |
uses: actions/setup-java@v4 |
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.
Ah great thank you, I'll make a follow up PR for this.
What does this change?
This upgrade the version of java that the lambdas run on to Java 21 as per our departmental recommendations
How to test
This has been regression tested on CODE
How can we measure success?
Have we considered potential risks?
Images
Accessibility