Dinesh Bhat has posted comments on this change.
Change subject: [tools] Implement a manual leader_step_down for a tablet
Patch Set 9:
> (5 comments)
> There is one extra instance of "master_addresses" string literal
> and corresponding description in tool_action_test.cc. Could you
> updated that to use the common constant as well?
TFTR, that instance was missing since I hadn't rebased on top of your latest
tool change I think, rebased and updated now.
PS9, Line 102: hostname:port
> Nit: do you want to add that the 'port' part could be omitted if the master
I guess so, although I would like to punt this into another patch after making
sure all the RPCs used by the toolset comply to that theory.
PS9, Line 104: I
> Nit: Is capitalization of 'i' intentional?
Sort of. I think we could be bit more consistent here; this was more in the
spirit of 'Tablet Server, Kudu Master'.. and so on although comparison isn't
really apple-to-apple. We have also used 'block identifier' to counter my
PS9, Line 81: extern
> Does 'extern' have to be present due to some compilation issues? I would e
Fixed the ordering. About 'extern', I remember looking up cpp guideline for
this, since I couldn't find anything, I followed the existing code's policy.
Besides, explicit keyword provides a really strong readability.
PS9, Line 175: string tablet_id
> nit: const string& tablet_id ?
Line 246: "to step down")
> nit: parameter/line continuation shift is 4 spaces.
I am not clear about this. Guideline seems to say : "start the arguments on a
new line indented by four spaces and continue at that 4 space indent". This
isn't necessarily starting the argument on a newline. I remember following what
other files have followed here, for eg take a look at tool_action_*.cc. If
needed correction, I can fix them all in this change or a different change.
To view, visit http://gerrit.cloudera.org:8080/4533
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Owner: Dinesh Bhat <din...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <d...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dral...@apache.org>
Gerrit-Reviewer: Dinesh Bhat <din...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>