Alexey Serbin has posted comments on this change. Change subject: [tools] Implement a manual leader_step_down for a tablet ......................................................................
Patch Set 8: (5 comments) http://gerrit.cloudera.org:8080/#/c/4533/8/src/kudu/tools/kudu-admin-test.cc File src/kudu/tools/kudu-admin-test.cc: Line 227: Status s = GetTermFromConsensus(tservers, tablet_id_, Nit: since the variable 's' is not used anywhere else down the code, consider ASSERT_TRUE(GetTermFromConsensus(...).IsNotFound()); http://gerrit.cloudera.org:8080/#/c/4533/8/src/kudu/tools/tool_action_local_replica.cc File src/kudu/tools/tool_action_local_replica.cc: PS8, Line 121: I Is capitalization of 'i' intentional? http://gerrit.cloudera.org:8080/#/c/4533/8/src/kudu/tools/tool_action_table.cc File src/kudu/tools/tool_action_table.cc: PS8, Line 53: const char* const kMasterAddressesArgDesc It seems this is used across different set of tools. Is it possible to define it only once, not repeating it over and over again across all those files which require this parameter? http://gerrit.cloudera.org:8080/#/c/4533/8/src/kudu/tools/tool_action_tablet.cc File src/kudu/tools/tool_action_tablet.cc: PS8, Line 67: I I saw this string is defined in different file as well. Does it make sense to have it defined only in a single place? E.g., putting it into kudu::tools::util namespace or alike. PS8, Line 177: string const string& here and further when using FindOrDie() ? The idea is that FindOrDie() returns const reference, and the result is not modified in the code below. -- To view, visit http://gerrit.cloudera.org:8080/4533 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia046a28a2008f4f5d1e955f57752a32a1ddc5ab8 Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Dinesh Bhat <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
