Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/8586 )
Change subject: WIP [client] expose non-voter replicas for kudu CLI tool ...................................................................... Patch Set 1: (4 comments) Looks alright to me until we can figure out how we want to expose this in the public client API http://gerrit.cloudera.org:8080/#/c/8586/1/src/kudu/client/meta_cache.h File src/kudu/client/meta_cache.h: http://gerrit.cloudera.org:8080/#/c/8586/1/src/kudu/client/meta_cache.h@372 PS1, Line 372: bool show_non_voters = false perhaps make this an enum? SHOW_NON_VOTERS / HIDE_NON_VOTERS Also this doesn't need to be explicit and it's probably best to get rid of the default value in the constructor http://gerrit.cloudera.org:8080/#/c/8586/1/src/kudu/tools/ksck_remote.cc File src/kudu/tools/ksck_remote.cc: http://gerrit.cloudera.org:8080/#/c/8586/1/src/kudu/tools/ksck_remote.cc@318 PS1, Line 318: show_non_voters_ might be a little friendlier to make this a setter method http://gerrit.cloudera.org:8080/#/c/8586/1/src/kudu/tools/tool_action_table.cc File src/kudu/tools/tool_action_table.cc: http://gerrit.cloudera.org:8080/#/c/8586/1/src/kudu/tools/tool_action_table.cc@64 PS1, Line 64: show_non_voters_ it would be less hacky to use a setter on Data for this http://gerrit.cloudera.org:8080/#/c/8586/1/src/kudu/tools/tool_action_table.cc@87 PS1, Line 87: is_voter_ mind putting a getter on this? -- 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: 1 Gerrit-Owner: 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: Fri, 17 Nov 2017 08:55:15 +0000 Gerrit-HasComments: Yes
