Adar Dembo has posted comments on this change. Change subject: Add basic Hive MetaStore client ......................................................................
Patch Set 8: (7 comments) http://gerrit.cloudera.org:8080/#/c/7053/8/CMakeLists.txt File CMakeLists.txt: Line 1144: add_subdirectory(src/kudu/hms) Nit: resort. http://gerrit.cloudera.org:8080/#/c/7053/8/src/kudu/hms/mini_hms.cc File src/kudu/hms/mini_hms.cc: Line 64: return Status::IllegalState(Substitute("$0 directory does not exist", env_var), *home_dir); Status::NotFound() seems more appropriate here. http://gerrit.cloudera.org:8080/#/c/7053/8/src/kudu/security/test/mini_kdc.cc File src/kudu/security/test/mini_kdc.cc: Line 29: Can you check whether any of these includes are no longer needed now that you've removed those functions from this file? http://gerrit.cloudera.org:8080/#/c/7053/8/thirdparty/build-definitions.sh File thirdparty/build-definitions.sh: Line 702: cmake \ Did you explicitly enumerate all BUILD/WITH options? Or are all of these non-default values? Line 714: -DWITH_OPENSSL=OFF \ Does this disable TLS support altogether? Line 717: -DBUILD_EXAMPLES=OFF \ Nit: resort. http://gerrit.cloudera.org:8080/#/c/7053/8/thirdparty/download-thirdparty.sh File thirdparty/download-thirdparty.sh: Line 325: if [ ! -d "$BISON_SOURCE" ]; then Could you add a comment here saying that we could run autoreconf but it doesn't seem to work properly on various platforms, so we won't? -- To view, visit http://gerrit.cloudera.org:8080/7053 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Jean-Daniel Cryans <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
