Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/12141 )
Change subject: KUDU-2411: Add OS/Arch detection to binary extract ...................................................................... Patch Set 5: (3 comments) Looking good, I just have some minor comments http://gerrit.cloudera.org:8080/#/c/12141/5/java/kudu-test-utils/src/main/java/org/apache/kudu/test/cluster/KuduBinaryLocator.java File java/kudu-test-utils/src/main/java/org/apache/kudu/test/cluster/KuduBinaryLocator.java: http://gerrit.cloudera.org:8080/#/c/12141/5/java/kudu-test-utils/src/main/java/org/apache/kudu/test/cluster/KuduBinaryLocator.java@88 PS5, Line 88: "Set the system variable " + KUDU_BIN_DIR_PROP + please update this message to include something like: "Set the system variable " + KUDU_BIN_DIR_PROP + " or add the Kudu binary test jar to your classpath or ensure the `kudu` binary is on your path." http://gerrit.cloudera.org:8080/#/c/12141/5/java/kudu-test-utils/src/test/java/org/apache/kudu/test/cluster/TestKuduBinaryJarExtractor.java File java/kudu-test-utils/src/test/java/org/apache/kudu/test/cluster/TestKuduBinaryJarExtractor.java: http://gerrit.cloudera.org:8080/#/c/12141/5/java/kudu-test-utils/src/test/java/org/apache/kudu/test/cluster/TestKuduBinaryJarExtractor.java@93 PS5, Line 93: customize nit: Just a style guide thing, please capitalize the first word in the sentence in a comment. Also please end the sentence with punctuation such as a period. http://gerrit.cloudera.org:8080/#/c/12141/5/java/kudu-test-utils/src/test/resources/log4j.properties File java/kudu-test-utils/src/test/resources/log4j.properties: http://gerrit.cloudera.org:8080/#/c/12141/5/java/kudu-test-utils/src/test/resources/log4j.properties@24 PS5, Line 24: log4j.logger.com.google.gradle = INFO Please add a comment as to why this is necessary / useful -- To view, visit http://gerrit.cloudera.org:8080/12141 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9752914a426dd1572610f891dad4d4778d04f79c Gerrit-Change-Number: 12141 Gerrit-PatchSet: 5 Gerrit-Owner: Brian McDevitt <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Brian McDevitt <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Comment-Date: Fri, 25 Jan 2019 07:32:23 +0000 Gerrit-HasComments: Yes
