Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/10907 )
Change subject: Add support for running Java tests in dist-test ...................................................................... Patch Set 5: (5 comments) http://gerrit.cloudera.org:8080/#/c/10907/5/build-support/run_dist_test.py File build-support/run_dist_test.py: http://gerrit.cloudera.org:8080/#/c/10907/5/build-support/run_dist_test.py@42 PS5, Line 42: read use readlines here instead of having to split('\n') below http://gerrit.cloudera.org:8080/#/c/10907/5/build-support/run_dist_test.py@44 PS5, Line 44: JAVA_CANDIDATES = [x for x in JAVA_CANDIDATES if not x.startswith("#")] maybe just combine the 'if not' part into the above comprehension? Mind adding the following to make sure that people don't try to put trailing comments into their list of paths? for c in JAVA_CANDIDATES: assert '#' not in c http://gerrit.cloudera.org:8080/#/c/10907/5/build-support/run_dist_test.py@101 PS5, Line 101: print >>sys.stderr, "found JAVA_HOME: " + x if you use logging, you'll get timestamps, plus be python-3 compatible. Or just use the print statement with () -- there is some "future" import you have to do to do that, though http://gerrit.cloudera.org:8080/#/c/10907/5/build-support/run_dist_test.py@112 PS5, Line 112: p.add_option("--test-type", dest="test_type", action="store", maybe test_language is better? http://gerrit.cloudera.org:8080/#/c/10907/5/cmake_modules/FindJavaHome.cmake File cmake_modules/FindJavaHome.cmake: http://gerrit.cloudera.org:8080/#/c/10907/5/cmake_modules/FindJavaHome.cmake@29 PS5, Line 29: #if (DEFINED ENV{JAVA_HOME}) why's this commented out? -- To view, visit http://gerrit.cloudera.org:8080/10907 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I446a15192a45e296b323a4c7d305f236e22ab557 Gerrit-Change-Number: 10907 Gerrit-PatchSet: 5 Gerrit-Owner: Grant Henke <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Comment-Date: Tue, 17 Jul 2018 01:43:15 +0000 Gerrit-HasComments: Yes
