Alexey Serbin has posted comments on this change.

Change subject: tool: better handling for positional arguments

Patch Set 1: Code-Review+1


There is a Go library which allows to implement rich CLI tools.  If using the 
library it's really easy to document the commands/sub-commands/parameters and 
it provides tab-completion support for shell as an additional feature.  It 
might be useful to take a look at that:

There is even more powerful library named Cobra:

We implemented very decent command-line tool using codegangsta/cli library in 
one of prior projects.

Also, if talking about command-line tools: given the abundance of CLI 
libraries, it might make sense to consider an option to implement those in Go.  
It's really fast and simple in development and seems to be the perfect language 
choice for that sort of things.
File src/kudu/tools/

PS1, Line 54: int
Nit: would you mind to use string::size_type instead of int for first_dash_idx 

PS1, Line 164: int
string::size_type ?
File src/kudu/tools/tool_action.h:

Line 21: #include <glog/logging.h>
Nit: if I'm not mistaken, by the code style the boost and glog headers should 
come after STL headers and before kudu-specific ones.

Line 162:   std::vector<std::string> optional;
Does it make sense to have description for an optional parameter as well?  As I 
see, both required and variadic ones have descriptions.
File src/kudu/tools/

Line 60:   const ActionArgDescriptor* args = &action->args();
What are benefits of using a pointer instead of a reference here?  Is it 
because operator-> is overloaded for the operand?  I.e. why not

const ActionArgDescriptor& args = action->args();

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic6aa517ccf1915e4d106bcc5f80a3abfeae03271
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <>
Gerrit-Reviewer: Alexey Serbin <>
Gerrit-Reviewer: Dan Burkert <>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <>
Gerrit-HasComments: Yes

Reply via email to