You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Grant Henke (Code Review)" <ge...@cloudera.org> on 2017/03/31 14:45:19 UTC
[kudu-CR] KUDU-1708. Document Kudu command-line tools
Grant Henke has uploaded a new change for review.
http://gerrit.cloudera.org:8080/6525
Change subject: KUDU-1708. Document Kudu command-line tools
......................................................................
KUDU-1708. Document Kudu command-line tools
- Adds helpxml flag support to the kudu binary
- Adds doc script and xslt to convert xml to asciidoc
Change-Id: I9f484f772cbaeb385687d83a2665ae4d7292aaf5
---
A docs/command_line_tools_reference.adoc
M docs/support/scripts/make_docs.sh
A docs/support/xsl/tool_to_asciidoc.xsl
M src/kudu/tools/tool_action.cc
M src/kudu/tools/tool_action.h
M src/kudu/tools/tool_main.cc
6 files changed, 356 insertions(+), 20 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/25/6525/1
--
To view, visit http://gerrit.cloudera.org:8080/6525
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I9f484f772cbaeb385687d83a2665ae4d7292aaf5
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
[kudu-CR] KUDU-1708. Document Kudu command-line tools
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: KUDU-1708. Document Kudu command-line tools
......................................................................
Patch Set 3:
(9 comments)
Any chance you could publish the generated docs in a GH branch so we can see how they render?
http://gerrit.cloudera.org:8080/#/c/6525/3/docs/support/scripts/make_docs.sh
File docs/support/scripts/make_docs.sh:
PS3, Line 173: # Reset environment to avoid affecting the default flag values.
: for var in $(env | awk -F= '{print $1}' | egrep -i 'KUDU|GLOG'); do
: echo "unset $var"
: eval "unset $var"
: done
Seems strange to do this here and for each binary above. Isn't just one call (before invoking any of the binaries) sufficient?
http://gerrit.cloudera.org:8080/#/c/6525/3/src/kudu/tools/tool_action.cc
File src/kudu/tools/tool_action.cc:
Line 31: #include <kudu/util/url-coding.h>
Should be double quotes, not triangular brackets.
Line 174: string Mode::BuildHelpXML(const vector<Mode*>& chain) const {
Should the XML be somewhat pretty printed, like with newlines and indentation? Otherwise it'll be pretty challenging to diagnose problems via human examination.
Line 182: for (const auto& a : actions()) {
Any particular reason to emit the actions before the modes? If it doesn't matter either way, maybe do modes before actions to be consistent with BuildHelp().
http://gerrit.cloudera.org:8080/#/c/6525/3/src/kudu/tools/tool_main.cc
File src/kudu/tools/tool_main.cc:
Line 146: string name = BinaryNameFromPath(argv[0]);
Can you just use BaseName() from util/path_util.h?
Line 155: exit(1);
It's a little unusual for a "helper" function like this to exit() directly. How about hoisting this call into main(), so it's more obvious?
Line 159: unique_ptr<Mode> root = RootMode(argv[0]);
Should use BinaryNameFromPath() (or BaseName()) here too.
Line 233: // Leverage existing helpxml flag to print mode/action xml
Nit: terminate with period.
Line 243: FLAGS_helpxml) {
The helpxml references can be removed from here and below.
--
To view, visit http://gerrit.cloudera.org:8080/6525
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f484f772cbaeb385687d83a2665ae4d7292aaf5
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1708. Document Kudu command-line tools
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6525
to look at the new patch set (#5).
Change subject: KUDU-1708. Document Kudu command-line tools
......................................................................
KUDU-1708. Document Kudu command-line tools
- Adds helpxml flag support to the kudu binary
- Adds doc script and xslt to convert xml to asciidoc
Change-Id: I9f484f772cbaeb385687d83a2665ae4d7292aaf5
---
A docs/command_line_tools_reference.adoc
M docs/support/scripts/make_docs.sh
A docs/support/xsl/tool_to_asciidoc.xsl
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action.cc
M src/kudu/tools/tool_action.h
M src/kudu/tools/tool_main.cc
7 files changed, 391 insertions(+), 23 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/25/6525/5
--
To view, visit http://gerrit.cloudera.org:8080/6525
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9f484f772cbaeb385687d83a2665ae4d7292aaf5
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
[kudu-CR] KUDU-1708. Document Kudu command-line tools
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6525
to look at the new patch set (#6).
Change subject: KUDU-1708. Document Kudu command-line tools
......................................................................
KUDU-1708. Document Kudu command-line tools
- Adds helpxml flag support to the kudu binary
- Adds doc script and xslt to convert xml to asciidoc
Change-Id: I9f484f772cbaeb385687d83a2665ae4d7292aaf5
---
A docs/command_line_tools_reference.adoc
M docs/support/scripts/make_docs.sh
A docs/support/xsl/tool_to_asciidoc.xsl
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action.cc
M src/kudu/tools/tool_action.h
M src/kudu/tools/tool_main.cc
7 files changed, 391 insertions(+), 23 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/25/6525/6
--
To view, visit http://gerrit.cloudera.org:8080/6525
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9f484f772cbaeb385687d83a2665ae4d7292aaf5
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
[kudu-CR] KUDU-1708. Document Kudu command-line tools
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6525
to look at the new patch set (#2).
Change subject: KUDU-1708. Document Kudu command-line tools
......................................................................
KUDU-1708. Document Kudu command-line tools
- Adds helpxml flag support to the kudu binary
- Adds doc script and xslt to convert xml to asciidoc
Change-Id: I9f484f772cbaeb385687d83a2665ae4d7292aaf5
---
A docs/command_line_tools_reference.adoc
M docs/support/scripts/make_docs.sh
A docs/support/xsl/tool_to_asciidoc.xsl
M src/kudu/tools/tool_action.cc
M src/kudu/tools/tool_action.h
M src/kudu/tools/tool_main.cc
6 files changed, 355 insertions(+), 20 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/25/6525/2
--
To view, visit http://gerrit.cloudera.org:8080/6525
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9f484f772cbaeb385687d83a2665ae4d7292aaf5
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
[kudu-CR] KUDU-1708. Document Kudu command-line tools
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change.
Change subject: KUDU-1708. Document Kudu command-line tools
......................................................................
Patch Set 7:
I made a few layout changes and published the generated docs here: https://granthenke.github.io/kudu/docs/command_line_tools_reference.html
--
To view, visit http://gerrit.cloudera.org:8080/6525
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f484f772cbaeb385687d83a2665ae4d7292aaf5
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: No
[kudu-CR] KUDU-1708. Document Kudu command-line tools
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6525
to look at the new patch set (#9).
Change subject: KUDU-1708. Document Kudu command-line tools
......................................................................
KUDU-1708. Document Kudu command-line tools
- Adds helpxml flag support to the kudu binary
- Adds doc script and xslt to convert xml to asciidoc
Change-Id: I9f484f772cbaeb385687d83a2665ae4d7292aaf5
---
A docs/command_line_tools_reference.adoc
M docs/support/jekyll-templates/document.html.erb
M docs/support/scripts/make_docs.sh
A docs/support/xsl/tool_to_asciidoc.xsl
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/kudu-tool-test.cc
A src/kudu/tools/tool_action-test.cc
M src/kudu/tools/tool_action.cc
M src/kudu/tools/tool_action.h
M src/kudu/tools/tool_main.cc
10 files changed, 496 insertions(+), 24 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/25/6525/9
--
To view, visit http://gerrit.cloudera.org:8080/6525
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9f484f772cbaeb385687d83a2665ae4d7292aaf5
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
[kudu-CR] KUDU-1708. Document Kudu command-line tools
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change.
Change subject: KUDU-1708. Document Kudu command-line tools
......................................................................
Patch Set 8:
> Any way to reduce the amount of whitespace between commands in the
> "command hierarchy" section of the doc? It seems quite wasteful.
I agree its a really spaced out bullet list. This must be default CSS since I don't do anything special.
--
To view, visit http://gerrit.cloudera.org:8080/6525
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f484f772cbaeb385687d83a2665ae4d7292aaf5
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: No
[kudu-CR] KUDU-1708. Document Kudu command-line tools
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change.
Change subject: KUDU-1708. Document Kudu command-line tools
......................................................................
Patch Set 8:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/6525/8/src/kudu/tools/CMakeLists.txt
File src/kudu/tools/CMakeLists.txt:
Line 31:
> I think your continuation line indentation is a little aggressive; should b
yeah, I need to fix my defaults.
Line 33: tool_action.cc)
> How about just adding this to the existing kudu_tools_util? Why do we need
Done
Line 121: ADD_KUDU_TEST(tool-action-test)
> Nit: should be tool_action-test (format is "<component>-test", and the comp
Done
http://gerrit.cloudera.org:8080/#/c/6525/8/src/kudu/tools/tool-action-test.cc
File src/kudu/tools/tool-action-test.cc:
Line 22: #include "gtest/gtest.h"
> Though counter-intuitive, these are external to the Kudu project so we trea
Done
Line 35: using std::unique_ptr;
> Should include <memory> for this.
Done
Line 58: "<description>rpd</description><type>string</type></argument>";
> Nit: indent a little more so it's aligned with the open quote from the line
Done
--
To view, visit http://gerrit.cloudera.org:8080/6525
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f484f772cbaeb385687d83a2665ae4d7292aaf5
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1708. Document Kudu command-line tools
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: KUDU-1708. Document Kudu command-line tools
......................................................................
Patch Set 7:
The default values emitted are all "None", probably a bug?
--
To view, visit http://gerrit.cloudera.org:8080/6525
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f484f772cbaeb385687d83a2665ae4d7292aaf5
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: No
[kudu-CR] KUDU-1708. Document Kudu command-line tools
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6525
to look at the new patch set (#3).
Change subject: KUDU-1708. Document Kudu command-line tools
......................................................................
KUDU-1708. Document Kudu command-line tools
- Adds helpxml flag support to the kudu binary
- Adds doc script and xslt to convert xml to asciidoc
Change-Id: I9f484f772cbaeb385687d83a2665ae4d7292aaf5
---
A docs/command_line_tools_reference.adoc
M docs/support/scripts/make_docs.sh
A docs/support/xsl/tool_to_asciidoc.xsl
M src/kudu/tools/tool_action.cc
M src/kudu/tools/tool_action.h
M src/kudu/tools/tool_main.cc
6 files changed, 355 insertions(+), 20 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/25/6525/3
--
To view, visit http://gerrit.cloudera.org:8080/6525
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9f484f772cbaeb385687d83a2665ae4d7292aaf5
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
[kudu-CR] KUDU-1708. Document Kudu command-line tools
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change.
Change subject: KUDU-1708. Document Kudu command-line tools
......................................................................
Patch Set 3:
(9 comments)
I will publish the generated docs in a GH branch tomorrow. (For some reason the make_site.sh script won't run on mac.)
http://gerrit.cloudera.org:8080/#/c/6525/3/docs/support/scripts/make_docs.sh
File docs/support/scripts/make_docs.sh:
Line 177: done
> Seems strange to do this here and for each binary above. Isn't just one cal
I think its done this way so that the reset of environment variables is scoped only around the binary execution. Not sure how important that actually is, but is seams more safe.
http://gerrit.cloudera.org:8080/#/c/6525/3/src/kudu/tools/tool_action.cc
File src/kudu/tools/tool_action.cc:
Line 31: #include <kudu/util/url-coding.h>
> Should be double quotes, not triangular brackets.
Done
Line 174: string Mode::BuildHelpXML(const vector<Mode*>& chain) const {
> Should the XML be somewhat pretty printed, like with newlines and indentati
I had thought about this. But I saw that the existing helpxml implementations didn't pretty pretty completely. I also thought it was easy enough to pipe to xmlint if someone needed it formatted: `bin/kudu --helpxml | xmllint --format -`
Line 182: for (const auto& a : actions()) {
> Any particular reason to emit the actions before the modes? If it doesn't m
I did actions first due to proximity. It doesn't come up much but we have a couple cases where the tree is a couple modes deep and there are both actions and modes at the same level. I thought it would be best to print the actions you could perform at your current level first before diving deeper. Otherwise as you read the page from top to bottom you read an action for the deepest mode and then suddenly an action 1 level up is shown.
I agree on being consistent with the command line help. The issue is less problematic there since all the information isn't shown recursively.
http://gerrit.cloudera.org:8080/#/c/6525/3/src/kudu/tools/tool_main.cc
File src/kudu/tools/tool_main.cc:
Line 146: string name = BinaryNameFromPath(argv[0]);
> Can you just use BaseName() from util/path_util.h?
Yes, thank you.
Line 155: exit(1);
> It's a little unusual for a "helper" function like this to exit() directly.
I used the flags.cc DumpFlagsXML() function as an example for this. Many of the functions there used mainly in HandleCommonFlags() do this. Should we change those too?
Line 159: unique_ptr<Mode> root = RootMode(argv[0]);
> Should use BinaryNameFromPath() (or BaseName()) here too.
I did not want to change the existing behavior. Currently when the help prints it will show the complete path to the kudu binary. I do not want that for the docs xml usage strings, but left it the same for user help strings.
Happy to change it if including the full path was unintentional.
Line 233: // Leverage existing helpxml flag to print mode/action xml
> Nit: terminate with period.
Done
Line 243: FLAGS_helpxml) {
> The helpxml references can be removed from here and below.
Done
--
To view, visit http://gerrit.cloudera.org:8080/6525
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f484f772cbaeb385687d83a2665ae4d7292aaf5
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1708. Document Kudu command-line tools
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: KUDU-1708. Document Kudu command-line tools
......................................................................
Patch Set 8:
(6 comments)
Any way to reduce the amount of whitespace between commands in the "command hierarchy" section of the doc? It seems quite wasteful.
http://gerrit.cloudera.org:8080/#/c/6525/8/src/kudu/tools/CMakeLists.txt
File src/kudu/tools/CMakeLists.txt:
Line 31:
I think your continuation line indentation is a little aggressive; should be 2 chars.
Line 33: tool_action.cc)
How about just adding this to the existing kudu_tools_util? Why do we need to introduce another target?
Line 121: ADD_KUDU_TEST(tool-action-test)
Nit: should be tool_action-test (format is "<component>-test", and the component is tool_action in this case).
http://gerrit.cloudera.org:8080/#/c/6525/8/src/kudu/tools/tool-action-test.cc
File src/kudu/tools/tool-action-test.cc:
PS8, Line 21: #include "gflags/gflags.h"
: #include "gtest/gtest.h"
Though counter-intuitive, these are external to the Kudu project so we treat them as system includes and use triangular brackets when including them.
Line 35: using std::unique_ptr;
Should include <memory> for this.
Line 58: "<description>rpd</description><type>string</type></argument>";
Nit: indent a little more so it's aligned with the open quote from the line before?
Below too.
--
To view, visit http://gerrit.cloudera.org:8080/6525
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f484f772cbaeb385687d83a2665ae4d7292aaf5
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1708. Document Kudu command-line tools
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change.
Change subject: KUDU-1708. Document Kudu command-line tools
......................................................................
Patch Set 7:
> The default values emitted are all "None", probably a bug?
Good catch. I fixed that and updated the generated docs.
--
To view, visit http://gerrit.cloudera.org:8080/6525
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f484f772cbaeb385687d83a2665ae4d7292aaf5
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: No
[kudu-CR] KUDU-1708. Document Kudu command-line tools
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: KUDU-1708. Document Kudu command-line tools
......................................................................
Patch Set 3:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/6525/3/docs/support/scripts/make_docs.sh
File docs/support/scripts/make_docs.sh:
Line 177: done
> I think its done this way so that the reset of environment variables is sco
Oh, I missed the subshell (parens) that all this is inside of. Never mind.
http://gerrit.cloudera.org:8080/#/c/6525/3/src/kudu/tools/tool_action.cc
File src/kudu/tools/tool_action.cc:
Line 182: for (const auto& a : actions()) {
> I did actions first due to proximity. It doesn't come up much but we have a
Makes sense; thanks for the explanation.
http://gerrit.cloudera.org:8080/#/c/6525/3/src/kudu/tools/tool_main.cc
File src/kudu/tools/tool_main.cc:
Line 155: exit(1);
> I used the flags.cc DumpFlagsXML() function as an example for this. Many of
Probably not, those functions are typically called far away from main(), so it'd be challenging to get main() to call exit(). Here, on the other hand, a special case for FLAGS_helpxml is carved out in main(), so it's easy to slip in exit().
Line 159: unique_ptr<Mode> root = RootMode(argv[0]);
> I did not want to change the existing behavior. Currently when the help pri
Hmm. It was unintentional, but I suppose having the full path in the usage is useful when working with an interactive shell (since you can execute the usage string almost verbatim).
http://gerrit.cloudera.org:8080/#/c/6525/6/src/kudu/tools/tool_main.cc
File src/kudu/tools/tool_main.cc:
Line 136: string::size_type path_tail = path.find_last_of("/\\");
This is now unused?
--
To view, visit http://gerrit.cloudera.org:8080/6525
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f484f772cbaeb385687d83a2665ae4d7292aaf5
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1708. Document Kudu command-line tools
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6525
to look at the new patch set (#7).
Change subject: KUDU-1708. Document Kudu command-line tools
......................................................................
KUDU-1708. Document Kudu command-line tools
- Adds helpxml flag support to the kudu binary
- Adds doc script and xslt to convert xml to asciidoc
Change-Id: I9f484f772cbaeb385687d83a2665ae4d7292aaf5
---
A docs/command_line_tools_reference.adoc
M docs/support/jekyll-templates/document.html.erb
M docs/support/scripts/make_docs.sh
A docs/support/xsl/tool_to_asciidoc.xsl
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action.cc
M src/kudu/tools/tool_action.h
M src/kudu/tools/tool_main.cc
8 files changed, 379 insertions(+), 23 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/25/6525/7
--
To view, visit http://gerrit.cloudera.org:8080/6525
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9f484f772cbaeb385687d83a2665ae4d7292aaf5
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
[kudu-CR] KUDU-1708. Document Kudu command-line tools
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: KUDU-1708. Document Kudu command-line tools
......................................................................
Patch Set 9: Code-Review+2 Verified+1
The Jenkins failure was a flake.
--
To view, visit http://gerrit.cloudera.org:8080/6525
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f484f772cbaeb385687d83a2665ae4d7292aaf5
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: No
[kudu-CR] KUDU-1708. Document Kudu command-line tools
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6525
to look at the new patch set (#8).
Change subject: KUDU-1708. Document Kudu command-line tools
......................................................................
KUDU-1708. Document Kudu command-line tools
- Adds helpxml flag support to the kudu binary
- Adds doc script and xslt to convert xml to asciidoc
Change-Id: I9f484f772cbaeb385687d83a2665ae4d7292aaf5
---
A docs/command_line_tools_reference.adoc
M docs/support/jekyll-templates/document.html.erb
M docs/support/scripts/make_docs.sh
A docs/support/xsl/tool_to_asciidoc.xsl
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/kudu-tool-test.cc
A src/kudu/tools/tool-action-test.cc
M src/kudu/tools/tool_action.cc
M src/kudu/tools/tool_action.h
M src/kudu/tools/tool_main.cc
10 files changed, 501 insertions(+), 24 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/25/6525/8
--
To view, visit http://gerrit.cloudera.org:8080/6525
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9f484f772cbaeb385687d83a2665ae4d7292aaf5
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
[kudu-CR] KUDU-1708. Document Kudu command-line tools
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has submitted this change and it was merged.
Change subject: KUDU-1708. Document Kudu command-line tools
......................................................................
KUDU-1708. Document Kudu command-line tools
- Adds helpxml flag support to the kudu binary
- Adds doc script and xslt to convert xml to asciidoc
Change-Id: I9f484f772cbaeb385687d83a2665ae4d7292aaf5
Reviewed-on: http://gerrit.cloudera.org:8080/6525
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Adar Dembo <ad...@cloudera.com>
---
A docs/command_line_tools_reference.adoc
M docs/support/jekyll-templates/document.html.erb
M docs/support/scripts/make_docs.sh
A docs/support/xsl/tool_to_asciidoc.xsl
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/kudu-tool-test.cc
A src/kudu/tools/tool_action-test.cc
M src/kudu/tools/tool_action.cc
M src/kudu/tools/tool_action.h
M src/kudu/tools/tool_main.cc
10 files changed, 496 insertions(+), 24 deletions(-)
Approvals:
Adar Dembo: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/6525
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I9f484f772cbaeb385687d83a2665ae4d7292aaf5
Gerrit-PatchSet: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
[kudu-CR] KUDU-1708. Document Kudu command-line tools
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change.
Change subject: KUDU-1708. Document Kudu command-line tools
......................................................................
Patch Set 8:
> Any way to reduce the amount of whitespace between commands in the
> "command hierarchy" section of the doc? It seems quite wasteful.
It turns out Asciidoc is adding paragraph tags to the list elements causing the extra space. I found this related issue as well: https://github.com/asciidoctor/asciidoctor/issues/1087
--
To view, visit http://gerrit.cloudera.org:8080/6525
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f484f772cbaeb385687d83a2665ae4d7292aaf5
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: No
[kudu-CR] KUDU-1708. Document Kudu command-line tools
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6525
to look at the new patch set (#4).
Change subject: KUDU-1708. Document Kudu command-line tools
......................................................................
KUDU-1708. Document Kudu command-line tools
- Adds helpxml flag support to the kudu binary
- Adds doc script and xslt to convert xml to asciidoc
Change-Id: I9f484f772cbaeb385687d83a2665ae4d7292aaf5
---
A docs/command_line_tools_reference.adoc
M docs/support/scripts/make_docs.sh
A docs/support/xsl/tool_to_asciidoc.xsl
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action.cc
M src/kudu/tools/tool_action.h
M src/kudu/tools/tool_main.cc
7 files changed, 390 insertions(+), 20 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/25/6525/4
--
To view, visit http://gerrit.cloudera.org:8080/6525
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9f484f772cbaeb385687d83a2665ae4d7292aaf5
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot