Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/22374 )

Change subject: Upgrade Java dependencies
......................................................................


Patch Set 17:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/22374/17//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/22374/17//COMMIT_MSG@44
PS17, Line 44: so this file
             : could be used in future when we compile with JDK 9
I'm not sure the placement of that file makes sense at all: the 
module-info.class must be in the root of a JAR, not under META-INF/versions/9.

Also, why other 9-versioned files in that MRJAR file aren't triggering any 
checks then?  I'm not sure I understand what's going on.  Could you shed some 
light on this, please?


http://gerrit.cloudera.org:8080/#/c/22374/17/build-support/verify_jars.pl
File build-support/verify_jars.pl:

http://gerrit.cloudera.org:8080/#/c/22374/17/build-support/verify_jars.pl@62
PS17, Line 62: META-INF/versions/9/module-info.class
Isn't this a fluke/bug?  Why module-info.class is under the versioned MRJAR 
section, not just under the root of the JAR file structure?

Did you check other published versions of micrometer JAR -- do they have the 
same strange placement of module-info.class due to some bug in the javac that 
created the JAR or that's something else behind?


http://gerrit.cloudera.org:8080/#/c/22374/17/java/kudu-hive/build.gradle
File java/kudu-hive/build.gradle:

http://gerrit.cloudera.org:8080/#/c/22374/17/java/kudu-hive/build.gradle@26
PS17, Line 26: testImplementation
Are you sure that's not needed in regular (i.e. not just test) runtime?

In other words, I'd have no question on this change if in prior version that 
was testCompileOnly and now you change it into 'testImplemenation' that adds it 
into the test run-time paths in addition to having it in compile-time path.  
But changing 'compileOnly' --> 'testImplementation' is a bit strange.



--
To view, visit http://gerrit.cloudera.org:8080/22374
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id1b43e3cc8228e94fbbd3085933cd62bf089e23d
Gerrit-Change-Number: 22374
Gerrit-PatchSet: 17
Gerrit-Owner: Abhishek Chennaka <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Attila Bukor <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Zoltan Chovan <[email protected]>
Gerrit-Comment-Date: Wed, 26 Feb 2025 20:26:05 +0000
Gerrit-HasComments: Yes

Reply via email to