Todd Lipcon has posted comments on this change.

Change subject: master_rpc: pass back more details from ConnectToCluster
......................................................................


Patch Set 1:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/5892/1/src/kudu/client/client-internal.cc
File src/kudu/client/client-internal.cc:

Line 606:     const master::ConnectToMasterResponsePB& connect_response) {
> warning: parameter 'connect_response' is unused [misc-unused-parameters]
Done


Line 607:   Status new_status = status;
> warning: local copy 'new_status' of the variable 'status' is never modified
Done


http://gerrit.cloudera.org:8080/#/c/5892/1/src/kudu/client/master_rpc.cc
File src/kudu/client/master_rpc.cc:

Line 273:     user_cb_(status, {}, {});
> Interesting. Didn't know an empty initializer list could be used to call th
yea, more convenient than typing out the class name.


http://gerrit.cloudera.org:8080/#/c/5892/1/src/kudu/client/master_rpc.h
File src/kudu/client/master_rpc.h:

PS1, Line 119:   // The index of the master that was determined to be the 
leader.
             :   // -1 indicates no leader found.
> Index in addrs_ and responses_? May want to make that explicit.
Done


http://gerrit.cloudera.org:8080/#/c/5892/1/src/kudu/integration-tests/external_mini_cluster.cc
File src/kudu/integration-tests/external_mini_cluster.cc:

PS1, Line 476:                 const Sockaddr& leader_master,
             :                 const master::ConnectToMasterResponsePB& resp) {
> Nit: indentation here?
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/5892
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I3b5056c9f31237249516a2e7ae68d641c9f6bd02
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: Dan Burkert <danburk...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-HasComments: Yes

Reply via email to