findepi commented on code in PR #10474:
URL: https://github.com/apache/iceberg/pull/10474#discussion_r1640228930
##########
baseline.gradle:
##########
@@ -60,8 +63,7 @@ subprojects {
spotless {
java {
target 'src/main/java/**/*.java', 'src/test/java/**/*.java',
'src/jmh/java/**/*.java', 'src/integration/java/**/*.java'
- // we use an older version of google-java-format that is compatible
with JDK 8
- googleJavaFormat("1.7")
+ googleJavaFormat("1.17.0")
Review Comment:
Thanks for rephrasing your initial comment for me. This reinforces my
understanding.
> so that spotlessApply doesn't produce different formatting results
depending on the JDK version it is executed with.
I totally agree this wouldn't be acceptable! Do you maybe remember under
what circumstances this can be observed?
In this PR I modified the CI scripts so that `./gradlew -DallVersions build
-x test -x javadoc -x integrationTest` runs with java 11-21 and all runs agree
on the desired formatting. While I can see that formatting depends on GJF
version, I don't see it being dependent on the JDK version it is executed with.
> Since we release for JDK8, we need to use the GJF version that works with
JDK8.
I hear you on this, but would appreciate helping me understanding the why. I
still fail to see it.
Is it because we expect Iceberg contributors to locally run JDK 8? I am
concerned that JDK 8 is unsupported, unmaintained publicly and potentially not
safe to run, so would prefer not to force contributors to use it, even though
the **releases** target Java 8 compatibility.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
