Todd Lipcon has posted comments on this change. Change subject: tool: improve help output ......................................................................
Patch Set 1: (5 comments) http://gerrit.cloudera.org:8080/#/c/4038/1//COMMIT_MSG Commit Message: Line 9: - adds blank line in between usage summary and more detail > nit for consistency: adds --> add, or justify --> justifies below instead Done http://gerrit.cloudera.org:8080/#/c/4038/1/src/kudu/tools/tool_action.cc File src/kudu/tools/tool_action.cc: Line 94: std::stringstream msg; > nit: std::ostringstream since no input is assumed. Done PS1, Line 95: [<args>] > nit: if here we are using mandatory/optional notation with <>/[] brackets, Just copied this from git's usage output. I think we would use: '[--foo]' if "--foo" was the literal string that's optional, whereas here we're saying '[<args>'] because <args> itself is a place-holder for something else Line 96: msg << "<command> can be one of the following:\n"; > nit: " <command> ..." i.e. add a couple of spaces for better readability but then it would be lined up with the subcommands. Or you think I should indent the subcommands even more? Again copied from git's format: todd@todd-ThinkPad-T540p:~/git/kudu$ git usage: git [--version] [--help] [-C <path>] [-c name=value] [--exec-path[=<path>]] [--html-path] [--man-path] [--info-path] [-p|--paginate|--no-pager] [--no-replace-objects] [--bare] [--git-dir=<path>] [--work-tree=<path>] [--namespace=<name>] <command> [<args>] The most commonly used git commands are: add Add file contents to the index bisect Find by binary search the change that introduced a bug ... Line 109: for (const auto& p : lines) { > Nit: shouldn't it be 'l' for 'line'? Or does 'p' mean something else? was using 'p' for 'pair'. will rename lines to line_pairs and the variable to 'lp' -- To view, visit http://gerrit.cloudera.org:8080/4038 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0b4fc2da2b03edf8ce6b9998eeabd06a4fcd216d Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon <t...@apache.org> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes