Conversation
armanbilge
left a comment
armanbilge
left a comment
There was a problem hiding this comment.
Oh drat, sorry. My review was stuck in "pending" for a week 😅
Thanks for the PR. I had one thought.
| javaOutputVersionOption | ||
| case Some(n) => | ||
| log.warn(s"tlJdkRelease is ${n} but scala ${scalaVersion.value} require JDK 17") | ||
| Seq.empty |
There was a problem hiding this comment.
| Seq.empty | |
| Seq("-java-output-version", 17) |
I think returning Seq.empty here is the wrong behavior. Instead, we should bump the JDK release to 17.
To explain why: developers specify this flag so that the compiler prevents them from using JDK APIs that are too new. For example, APIs introduced in Java 21. Even if a JDK release version of 8 or 11 is no longer compatible with Scala 3.8+, we should still ensure that the compiler will prevent the use of APIs from JDKs >17.
In fact, I would even say the warning would just be noise and we can make this adjustment silently. By choosing to use Scala 3.8+, the developer is already opting-in to minimum JDK 17+.
2 participants
No description provided.