Dan Burkert has posted comments on this change.

Change subject: Add basic Hive MetaStore client
......................................................................


Patch Set 9:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/7053/8/CMakeLists.txt
File CMakeLists.txt:

Line 1144: add_subdirectory(src/kudu/hms)
> Nit: resort.
Done


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.
Done


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 y
Done


http://gerrit.cloudera.org:8080/#/c/7053/8/thirdparty/build-definitions.sh
File thirdparty/build-definitions.sh:

Line 702:     -DCMAKE_BUILD_TYPE=Release \
> Did you explicitly enumerate all BUILD/WITH options? Or are all of these no
Pretty much the former.  For whatever reason thrift defaults everything to ON, 
so I looked at the list of things it was building and turned everything off I 
didn't like.


Line 714:     -DWITH_PLUGIN=OFF \
> Does this disable TLS support altogether?
Yes, we may need to go back and turn this on later, but for now it's simpler if 
we don't need to tell thrift where to find OpenSSL.


Line 717:     -DBUILD_EXAMPLES=OFF \
> Nit: resort.
Done


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 doe
Done


-- 
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: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <danburk...@apache.org>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <danburk...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jdcry...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-HasComments: Yes

Reply via email to