Alexey Serbin 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?

http://gerrit.cloudera.org:8080/#/c/4533/9/src/kudu/tools/tool_action_common.cc
File src/kudu/tools/tool_action_common.cc:

PS9, Line 102: hostname:port
Nit: do you want to add that the 'port' part could be omitted if the master 
listens at the default port?


PS9, Line 104: I
Nit: Is capitalization of 'i' intentional?


http://gerrit.cloudera.org:8080/#/c/4533/9/src/kudu/tools/tool_action_common.h
File src/kudu/tools/tool_action_common.h:

PS9, Line 81: extern
Does 'extern' have to be present due to some compilation issues?  I would 
expect it to compile without 'extern'.

Also, consider moving those up to precede the first function in the kudu::tools 
namespace.  This is to be more conformant with the declaration order specified 
by the style guide: 
https://google.github.io/styleguide/cppguide.html#Declaration_Order


http://gerrit.cloudera.org:8080/#/c/4533/9/src/kudu/tools/tool_action_tablet.cc
File src/kudu/tools/tool_action_tablet.cc:

PS9, Line 175: string tablet_id
nit: const string& tablet_id ?


Line 246:         "to step down")
nit: parameter/line continuation shift is 4 spaces.


-- 
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: 9
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

Reply via email to