Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/10077 )
Change subject: [Java] Use Yetus annotations in place of @VisibleForTesting ...................................................................... Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/10077/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/10077/3//COMMIT_MSG@10 PS3, Line 10: This patch replaces Guava’s @VisibleForTesting with : @InterfaceAudience.LimitedPrivate("Test”) > How can we enforce that new @VisibleForTesting instances don't find their w I think we can blacklist modules from being used via checkstyle. I can make that change in a follow up patch, but currently checkstyle is too broken to enable failing the build IIRC. http://gerrit.cloudera.org:8080/#/c/10077/3//COMMIT_MSG@19 PS3, Line 19: eliminating any Guava use in kudu-spark, : kudu-spark-tools and kudu-hive which didn’t : have Gauva marked as a dependency > How can we enforce that Guava usage doesn't return? Clearly the missing dep We can be strict about using undeclared dependencies. Something we probably should do, but currently that would take a bit of clean up. We have a few areas where we pull in a "parent/bundle" dependency and use the underlying dependencies. There is a it of a gray area of when this is and isn't okay. I would be happy to make things more strict when we cut over to gradle. I would rather not do it in both Maven and Gradle. http://gerrit.cloudera.org:8080/#/c/10077/3/java/kudu-spark-tools/pom.xml File java/kudu-spark-tools/pom.xml: http://gerrit.cloudera.org:8080/#/c/10077/3/java/kudu-spark-tools/pom.xml@51 PS3, Line 51: <groupId>org.apache.yetus</groupId> > Not provided scope like for kudu-hive? kudu-hive I added provided scope because it broke the c++ build based on how the jars are discovered. But yes, Yetus could be provided scope as the annotations are not really needed/used at runtime. I had a separate patch changing all of the others I was going to post. But I should change this one now since I am introducing it. -- To view, visit http://gerrit.cloudera.org:8080/10077 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic0d8845e8cbbf5ea81345c46c5487121d1098f91 Gerrit-Change-Number: 10077 Gerrit-PatchSet: 3 Gerrit-Owner: Grant Henke <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Comment-Date: Mon, 16 Apr 2018 18:36:55 +0000 Gerrit-HasComments: Yes
