build: require java 17+ and update readme #2448
Conversation
settings.gradle.kts
Outdated
| @@ -22,6 +22,6 @@ plugins { | |||
| id("org.gradle.toolchains.foojay-resolver-convention") version ("0.3.0") | |||
| } | |||
|
|
|||
| if (JavaVersion.current() < JavaVersion.VERSION_11) { | |||
| throw GradleException("SpongeAPI requires at least Java 11 to build, but you have ${JavaVersion.current()}.") | |||
| if (JavaVersion.current() < JavaVersion.VERSION_17) { | |||
There was a problem hiding this comment.
The Java version required to run gradle does not necessarily have to be the Java version used to compile -- this is just for buildscript plugins (iirc originally checkstyle) to run.
There was a problem hiding this comment.
Should we optimize for the majority ? I'm thinking most people clone the repo to tinker/contribute so they probably want to build it. Any thoughts ?
There was a problem hiding this comment.
Well, you can build either way -- if you're running on an older Java version, Gradle will try to detect the Java 17 JVM or if necessary download it -- it's easier to be more lenient rather than making devs muck around with their environment variables.
There was a problem hiding this comment.
Ah, didn't know that. Followup question: what is the check used for ? Woudn't it be best to remove and let gradle handle the java version then ? I'm not sure why we are specifically enforcing 11
There was a problem hiding this comment.
I believe I added that check because newer versions of Checkstyle require J11 and Gradle didn't yet support toolchains on the checkstyle task at the time (it does now, so that could be configured to use the project toolchain instead and this check could be removed)
There was a problem hiding this comment.
I see we are not using the checkstyle plugin directly but we're relying on indra which has the toolchain version set (https://github.com/SpongePowered/SpongeAPI/blob/api-10/build.gradle.kts#L258). Shouldn't it take care of it ?
|
This does however bring up the issue of the CI running gradle with Java 11, which is a bit redundant since 17 will have to be downloaded by Gradle anyways. You should be able to add a |
Thanks, had to dig into the common workflows 😄 |
MC 1.18+ require java 17+
Update readme accordingly and replace old links