Alexey Serbin has posted comments on this change. Change subject: tool: better handling for positional arguments ......................................................................
Patch Set 1: Code-Review+1 (5 comments) 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: https://www.progville.com/go/cli-go-better-command-line-applications-go/ https://github.com/urfave/cli There is even more powerful library named Cobra: https://github.com/spf13/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. http://gerrit.cloudera.org:8080/#/c/4013/1/src/kudu/tools/tool_action.cc File src/kudu/tools/tool_action.cc: PS1, Line 54: int Nit: would you mind to use string::size_type instead of int for first_dash_idx variable? PS1, Line 164: int string::size_type ? http://gerrit.cloudera.org:8080/#/c/4013/1/src/kudu/tools/tool_action.h 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. http://gerrit.cloudera.org:8080/#/c/4013/1/src/kudu/tools/tool_main.cc File src/kudu/tools/tool_main.cc: 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 http://gerrit.cloudera.org:8080/4013 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic6aa517ccf1915e4d106bcc5f80a3abfeae03271 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Dan Burkert <d...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes
