Dan Burkert has posted comments on this change.

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


Patch Set 3:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/7053/2/src/kudu/hms/hive_metastore-test.cc
File src/kudu/hms/hive_metastore-test.cc:

Line 29: #include "kudu/hms/mini_hms.h"
> warning: #includes are not sorted properly [llvm-include-order]
Done


http://gerrit.cloudera.org:8080/#/c/7053/2/src/kudu/hms/hive_metastore.cc
File src/kudu/hms/hive_metastore.cc:

Line 87:   HMS_RET_NOT_OK(client_.create_database(db), "create Hive MetaStore 
database");
> warning: passing result of std::move() as a const reference argument; no mo
Done


http://gerrit.cloudera.org:8080/#/c/7053/2/src/kudu/hms/mini_hms.cc
File src/kudu/hms/mini_hms.cc:

Line 50: 
> warning: using decl 'vector' is unused [misc-unused-using-decls]
Done


http://gerrit.cloudera.org:8080/#/c/7053/2/src/kudu/util/subprocess.cc
File src/kudu/util/subprocess.cc:

Line 768:       break;
> warning: do not use 'else' after 'break' [readability-else-after-return]
I hate this rule.


Line 772:     }
> warning: either cast from 'int' to 'int64_t' (aka 'long') is ineffective, o
Done


http://gerrit.cloudera.org:8080/#/c/7053/2/src/kudu/util/subprocess.h
File src/kudu/util/subprocess.h:

Line 201: Status GetExecutablePath(const std::string& binary,
> warning: function 'kudu::GetExecutablePath' has a definition with different
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: 3
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

Reply via email to