gemini-code-assist[bot] commented on code in PR #37780:
URL: https://github.com/apache/beam/pull/37780#discussion_r2891402863
##########
sdks/java/io/hcatalog/build.gradle:
##########
@@ -53,24 +53,40 @@ dependencies {
provided library.java.jackson_databind
// Calcite (a dependency of Hive) bundles without repackaging Guava which is
why we redeclare it
// here so that it appears on the compile/test/runtime classpath before
Calcite.
+ provided library.java.joda_time
Review Comment:
The `joda_time` library is already declared as an `implementation`
dependency on line 48. Adding it again as a `provided` dependency on line 56
might be redundant unless there's a specific reason related to dependency
resolution order or classpath management for the `provided` scope that is not
immediately apparent. If the `implementation` declaration already covers the
need for `joda_time` during compilation, this line can be removed to avoid
redundancy.
##########
sdks/java/io/hcatalog/build.gradle:
##########
@@ -53,24 +53,40 @@ dependencies {
provided library.java.jackson_databind
// Calcite (a dependency of Hive) bundles without repackaging Guava which is
why we redeclare it
// here so that it appears on the compile/test/runtime classpath before
Calcite.
+ provided library.java.joda_time
provided library.java.hadoop_common
- provided "org.apache.hive:hive-exec:$hive_version"
+ provided(group: "org.apache.hive", name: "hive-exec", version: hive_version,
classifier: "core")
provided(group: "org.apache.hive.hcatalog", name: "hive-hcatalog-core",
version: hive_version) {
exclude group: "org.apache.hive", module: "hive-exec"
- exclude group: "com.google.protobuf", module: "protobuf-java"
}
+ testImplementation library.java.joda_time
Review Comment:
Similar to the `provided` declaration, `joda_time` is already an
`implementation` dependency on line 48. If the `implementation` dependency is
sufficient for test compilation and runtime, this `testImplementation`
declaration might be redundant. Please confirm if this explicit
`testImplementation` is necessary, perhaps due to specific test classpath
requirements or to override a transitive dependency.
##########
sdks/java/io/hcatalog/build.gradle:
##########
@@ -107,5 +123,10 @@ hadoopVersions.each { kv ->
description = "Runs HCatalog tests with Hadoop version $kv.value"
classpath = configurations."hadoopVersion$kv.key" +
sourceSets.test.runtimeClasspath
include '**/*Test.class'
+ jvmArgs "--add-opens=java.base/java.net=ALL-UNNAMED"
Review Comment:
The `jvmArgs` for `--add-opens` is already applied globally to all `Test`
tasks via the `tasks.withType(Test).configureEach` block starting on line 130.
This specific declaration on line 126 for `hadoopVersion${kv.key}Test` tasks
becomes redundant and can be removed.
--
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]
