Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/8586 )
Change subject: [client] expose non-voter replicas for kudu CLI tool ...................................................................... Patch Set 3: Code-Review+1 (3 comments) lgtm http://gerrit.cloudera.org:8080/#/c/8586/3/src/kudu/client/client.cc File src/kudu/client/client.cc: http://gerrit.cloudera.org:8080/#/c/8586/3/src/kudu/client/client.cc@a340 PS3, Line 340: Is there a reason for removing these parens? http://gerrit.cloudera.org:8080/#/c/8586/3/src/kudu/client/client.cc@550 PS3, Line 550: bool is_leader = r.role() == consensus::RaftPeerPB::LEADER; Would you mind adding a TODO comment to try to go back and use member_type with attributes instead of role for the meta cache and related client code? Role is starting to become an outdated concept as we evolve the system. http://gerrit.cloudera.org:8080/#/c/8586/3/src/kudu/client/replica_controller.h File src/kudu/client/replica_controller.h: http://gerrit.cloudera.org:8080/#/c/8586/3/src/kudu/client/replica_controller.h@27 PS3, Line 27: namespace internal { Based on recent convention, I think these files (.h and .cc) should also be named -internal.* -- To view, visit http://gerrit.cloudera.org:8080/8586 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I19317fdf5a2d5c8bb5f37b27bb83067a4df4ea20 Gerrit-Change-Number: 8586 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Will Berkeley <[email protected]> Gerrit-Comment-Date: Wed, 29 Nov 2017 04:04:52 +0000 Gerrit-HasComments: Yes
