-
Notifications
You must be signed in to change notification settings - Fork 11
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
Unify java versions #399
Unify java versions #399
Conversation
.github/workflows/ci.yml
Outdated
@@ -31,7 +31,7 @@ jobs: | |||
uses: actions/setup-java@v4 | |||
with: | |||
distribution: corretto | |||
java-version: '17' | |||
java-version: '11' |
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.
This was on purpose using Java17 because it's faster than Java11 and it's the same for all the 17
that you replaced with 11
in this file.
The important thing is that your tests are run on the 3 versions, which is already the case. See https://github.com/zio/zio-sbt/pull/399/files#diff-b803fcb7f17ed9235f1e5cb1fcd2f5d3b2838429d4368ae4c57ce4436577f03fR67-R77
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'll change the wording in setting key then because it's a bit confusing
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.
Fixed
@@ -109,7 +109,7 @@ object ZioSbtEcosystemPlugin extends AutoPlugin { | |||
scalaVersion := scala213.value, | |||
crossScalaVersions := Seq(scala212.value, scala213.value, scala3.value), | |||
zioVersion := Versions.zioVersion, | |||
javaPlatform := "8" | |||
javaPlatform := "11" |
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.
+1
Restore Java 17
@andrzejressel Thanks for your contribution 🙂 |
I was migrating zio-logging to latest version and I've noticed some inconsistencies regarding Java version. While most of them are minor the release one caught my eye.
This PR:
Sets default release Java to 11 - I believe libraries are released for Java 11 anyway, because of "source/target" flags, but it looks strange when docs are saying that Java 17 is default for release.I've added more descriptive commentShouldn'tjavacOptions
be removed altogether to ensure that tests on latest Java are actually using the new features?