Todd Lipcon has posted comments on this change. Change subject: Update kudu-lint to latest LLVM APIs ......................................................................
Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/4475/1/build-support/tools/kudu-lint/CMakeLists.txt File build-support/tools/kudu-lint/CMakeLists.txt: PS1, Line 22: # Find LLVM > This isn't actually getting LLVM/clang from thirdparty, is it? I think we'd You know, I'm not sure. It "just worked" on my machine and I dont really want to spend time to investigate it. At least this patch makes it more likely to work than it used to (which didn't work at all) PS1, Line 63: curses : dl : pthread : z > Should we do some find_library() or find_package() calls for these? I suppose so but again I dont think it's worth spending the time since this tool should probably go away. http://gerrit.cloudera.org:8080/#/c/4475/1/build-support/tools/kudu-lint/README File build-support/tools/kudu-lint/README: PS1, Line 28: 3.8 > Nit: can we drop or reword this? Otherwise it's stale once clang in thirdpa OK, I'll make a note that this tool isn't actively maintained and may not work with versions other than 3.8. (it seems to work with 3.9 for me as well though) PS1, Line 44: -extra-arg > Doesn't your change drop support for the -extra-arg argument? nope, it's actually part of the standard tooling infrastructure now. -- To view, visit http://gerrit.cloudera.org:8080/4475 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7c69be84cad56df71cc1bb2a3a89ada5d2167665 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon <t...@apache.org> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes
