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 <granthe...@apache.org>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Grant Henke <granthe...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-Comment-Date: Mon, 16 Apr 2018 18:36:55 +0000
Gerrit-HasComments: Yes

Reply via email to