Adar Dembo has posted comments on this change.
Change subject: tool: port kudu-admin to 'kudu cluster'
Patch Set 4:
Line 293: unique_ptr<Action> add_server =
> I missed that it's already sorted in BuildHelp, never mind then.
There's no sorting in Action::BuildHelp() or Mode::BuildHelp(). But aren't the
actions already sorted alphabetically? I've ordered them as "delete_table",
"ksck", and "list_tables"; that's alphabetical, no?
Line 91: const char* const kTabletIdArg = "tablet_id";
> nice, i can consolidate in kudu-test-tool as well like this, but why not co
It's a Google style guide convention:
PS4, Line 117: tablet_raw
> Bear with a basic Qn: what prevents us from using unique_ptr directly inste
I'm not aware of a way to "offer" the internal pointer stored by the unique_ptr
such that it can be passed to GetTablet() (which only accepts a pointer to a
pointer as an argument).
PS4, Line 177: kudu
> some const string again ? or GetBinName kinda routine if available ?
This is just a random sequence of characters; its value doesn't matter, and
since I'm not reusing it elsewhere, I've just inlined the value.
Line 204: return ChangeConfig(context, consensus::REMOVE_SERVER);
> I don't see you taking care of REMOVE_SERVER anywhere in ChangeConfig. In f
Check out L186: the cc_type is stored directly in the ChangeConfigRequestPB
class. REMOVE_SERVER is unique in that it doesn't need anything on top of that.
PS4, Line 303: VOTER
> This need not have been explicit if there are only two types - i.e, if the
Yeah, that's why I've set it up this way.
To view, visit http://gerrit.cloudera.org:8080/4180
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Owner: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com>
Gerrit-Reviewer: Dinesh Bhat <din...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mpe...@apache.org>
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>