Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/11686 )
Change subject: KUDU-2411: (Part 1) Break out existing test utilities into a seperate module ...................................................................... Patch Set 2: (5 comments) http://gerrit.cloudera.org:8080/#/c/11686/2/java/kudu-protocol/build.gradle File java/kudu-protocol/build.gradle: http://gerrit.cloudera.org:8080/#/c/11686/2/java/kudu-protocol/build.gradle@18 PS2, Line 18: apply from: "$rootDir/gradle/protobuf.gradle" > What's the purpose of this new module? Breaking out the protocol build is a bit of a side task now. It was needed at one point, but isn't required anymore. I kept it broke out because I liked the protobuf compiled classes being in a separate module/jar from the client code. It could be useful for protocol testing, or other implementations to use the protocol directly. http://gerrit.cloudera.org:8080/#/c/11686/2/java/kudu-protocol/build.gradle@35 PS2, Line 35: protcol > protocol Done http://gerrit.cloudera.org:8080/#/c/11686/2/java/kudu-test-utils/build.gradle File java/kudu-test-utils/build.gradle: http://gerrit.cloudera.org:8080/#/c/11686/2/java/kudu-test-utils/build.gradle@22 PS2, Line 22: > whitespace Done http://gerrit.cloudera.org:8080/#/c/11686/2/java/kudu-test-utils/build.gradle@34 PS2, Line 34: // kudu-test-utils has no public Javadoc. > Seems like it should? Because all of the classes are marked as InterfaceAudiance.Private the javadoc doclet filters them out of the docs. Once we make some things public we can remove this. http://gerrit.cloudera.org:8080/#/c/11686/2/java/kudu-test-utils/src/main/java/org/apache/kudu/test/cluster/MiniKuduCluster.java File java/kudu-test-utils/src/main/java/org/apache/kudu/test/cluster/MiniKuduCluster.java: http://gerrit.cloudera.org:8080/#/c/11686/2/java/kudu-test-utils/src/main/java/org/apache/kudu/test/cluster/MiniKuduCluster.java@179 PS2, Line 179: throw new IOException(resp.getError().getMessage()); > This should probably be RuntimeException, this is an error happening on the NonRecoverableException is package private. I could change that, but given that the end result is essentially an IOException that contains the response message I figured this was fine. -- To view, visit http://gerrit.cloudera.org:8080/11686 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifa935d6038b6d8756b332178347cec5cb70660a9 Gerrit-Change-Number: 11686 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 16 Oct 2018 14:35:02 +0000 Gerrit-HasComments: Yes
