-
Notifications
You must be signed in to change notification settings - Fork 4k
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
THRIFT-5584: use gradle toolchain to specify Java 11 with --release 8 #2606
Conversation
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.
Looks good to me, for the most part. I would suggest a slight change to the error message.
Also, there are some conditionals in the Gradle build files that check for Java 11. Those can be removed now.
We'd also need to remove the CI checks that run using JDK 8, like that the Ubuntu Xenial builds, unless you want to restore your previous workaround to force JDK 11 to be installed.
Co-authored-by: Christopher Tubbs <[email protected]>
we don't need to update the xenial build, since using java toolchain means we are decoupling the JDK used to run gradle from the one used to compile Java. in the case of xenial, JDK8 is used to run gradle, which will download JDK 11 on its own before compiling Java. |
Ah, ok. That's convenient. I assumed it worked like Maven toolchains: detected but not installed. |
yes there's a section in here describing the auto provisioning feature in case you are interested |
why not split dependency updates or other code changes - not related to Gradle/Java update from real Gradle Java 11 upgrade? |
If they are small and trivial, like these, I don't think it matters much. All of these are related to updating the build. |
Hi, It seems that the version 0.18 is compiled in Java 11 even if --release 8 is present :
|
@nicolasb29 That's not a bug. Thrift moved to Java 11 in #2686 (THRIFT-5644) |
use gradle toolchain to specify Java 11 with --release 8
[skip ci]
anywhere in the commit message to free up build resources.