Marton Greber has posted comments on this change. ( http://gerrit.cloudera.org:8080/24517 )
Change subject: [java] Upgrade build to Gradle 9 ...................................................................... Patch Set 3: (6 comments) http://gerrit.cloudera.org:8080/#/c/24517/3/java/build.gradle File java/build.gradle: http://gerrit.cloudera.org:8080/#/c/24517/3/java/build.gradle@109 PS3, Line 109: duplicatesStrategy = DuplicatesStrategy.INCLUDE nit: `INCLUDE` means when multiple subprojects contribute the same jar filename, all copies land in the output directory - last writer wins for same-name files. `EXCLUDE` (skip duplicates) would be deterministic and functionally equivalent here since same-named jars across subprojects should be identical transitive deps. Was `INCLUDE` chosen intentionally? http://gerrit.cloudera.org:8080/#/c/24517/3/java/config/spotbugs/excludeFilter.xml File java/config/spotbugs/excludeFilter.xml: http://gerrit.cloudera.org:8080/#/c/24517/3/java/config/spotbugs/excludeFilter.xml@32 PS3, Line 32: <Or> q: The global suppression of `AT_NONATOMIC_64BIT_PRIMITIVE`, `AT_NONATOMIC_OPERATIONS_ON_SHARED_VARIABLE`, and `AT_STALE_THREAD_WRITE_OF_PRIMITIVE` silences all concurrency warnings from SpotBugs project-wide. These are arguably the highest-value findings SpotBugs can produce. The comment says "slated for follow-up cleanup" - is there a tracking ticket for re-enabling these (worth putting it here)? http://gerrit.cloudera.org:8080/#/c/24517/3/java/gradle/quality.gradle File java/gradle/quality.gradle: http://gerrit.cloudera.org:8080/#/c/24517/3/java/gradle/quality.gradle@81 PS3, Line 81: spotbugs { q: The old config pinned `toolVersion = versions.spotBugs` (currently "4.2.2" in dependencies.gradle). After this change the SpotBugs tool version is governed by the plugin default (whatever 6.4.7 ships). The `versions.spotBugs` entry in dependencies.gradle is now stale/unused - should it be removed to avoid confusion? And is the implicit version bump from 4.2.2 to 4.8+ intentional or incidental? http://gerrit.cloudera.org:8080/#/c/24517/3/java/gradle/quality.gradle@83 PS3, Line 83: effort = Effort.valueOf("MAX") nit: Since you're already importing `Effort` and `Confidence` at the top of the file, why not use the enum constants directly (`Effort.MAX`, `Confidence.LOW`) instead of `valueOf("MAX")`? http://gerrit.cloudera.org:8080/#/c/24517/3/java/kudu-proto/build.gradle File java/kudu-proto/build.gradle: http://gerrit.cloudera.org:8080/#/c/24517/3/java/kudu-proto/build.gradle@45 PS3, Line 45: spotbugsMain.enabled = false nit: Directly accessing `spotbugsMain` eagerly realizes the task. The Gradle 9 idiom would be `tasks.named("spotbugsMain") { enabled = false }` to stay within the lazy task configuration model. Same for `spotbugsTest`. Minor since this module is small, but inconsistent with the `tasks.matching`/`tasks.named` style used elsewhere in this patch (e.g., shadow.gradle). http://gerrit.cloudera.org:8080/#/c/24517/3/java/kudu-subprocess/src/main/resources/META-INF/services/javax.ws.rs.ext.MessageBodyReader File java/kudu-subprocess/src/main/resources/META-INF/services/javax.ws.rs.ext.MessageBodyReader: http://gerrit.cloudera.org:8080/#/c/24517/3/java/kudu-subprocess/src/main/resources/META-INF/services/javax.ws.rs.ext.MessageBodyReader@1 PS3, Line 1: com.sun.jersey.core.impl.provider.entity.StringProvider q: These service-provider descriptors reference `com.sun.jersey.*` (Jersey 1.x). The commit message says they're needed for provider discovery "without classpath scanning." Was the old behavior relying on Jersey's internal classpath scanner that stopped working under the new Shadow/classloader setup? If so, a one-line comment at the top of each file explaining why they're needed would save the next person from removing them thinking they're dead code. -- To view, visit http://gerrit.cloudera.org:8080/24517 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I874d631ce7f4ca6527c9ffda6caef75c2976932d Gerrit-Change-Number: 24517 Gerrit-PatchSet: 3 Gerrit-Owner: Zoltan Chovan <[email protected]> Gerrit-Reviewer: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Ashwani Raina <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Gabriella Lotz <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber <[email protected]> Gerrit-Reviewer: Zoltan Chovan <[email protected]> Gerrit-Reviewer: Zoltan Martonka <[email protected]> Gerrit-Comment-Date: Mon, 29 Jun 2026 16:13:46 +0000 Gerrit-HasComments: Yes
