You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Adar Dembo (Code Review)" <ge...@cloudera.org> on 2016/08/28 17:54:49 UTC
[kudu-CR] tool: split up action descriptions
Hello Todd Lipcon,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/4148
to review the following change.
Change subject: tool: split up action descriptions
......................................................................
tool: split up action descriptions
With ksck we have a use case for "short" and "long" action descriptions: the
former appears in mode help and action help, while the latter is only in
action help. This patch splits action descriptions into "short" and "extra",
refactoring all actions accordingly. While I was there I got rid of the
label abstraction, which I felt was producing too much unfluent code.
I also added some more tests to kudu-tool-test.
Change-Id: Ie90b6228bba54575933fd9ac2f4f0bebf579ecd7
---
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_action_cluster.cc
M src/kudu/tools/tool_action_fs.cc
M src/kudu/tools/tool_action_pbc.cc
M src/kudu/tools/tool_action_tablet.cc
M src/kudu/tools/tool_main.cc
8 files changed, 255 insertions(+), 97 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/48/4148/1
--
To view, visit http://gerrit.cloudera.org:8080/4148
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie90b6228bba54575933fd9ac2f4f0bebf579ecd7
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] tool: split up action descriptions
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change.
Change subject: tool: split up action descriptions
......................................................................
Patch Set 4:
Build Started http://104.196.14.100/job/kudu-gerrit/3134/
--
To view, visit http://gerrit.cloudera.org:8080/4148
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie90b6228bba54575933fd9ac2f4f0bebf579ecd7
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] tool: split up action descriptions
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Hello Todd Lipcon, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/4148
to look at the new patch set (#2).
Change subject: tool: split up action descriptions
......................................................................
tool: split up action descriptions
With ksck we have a use case for "short" and "long" action descriptions: the
former appears in mode help and action help, while the latter is only in
action help. This patch splits action descriptions into "short" and "extra",
refactoring all actions accordingly. While I was there I got rid of the
label abstraction, which I felt was producing too much unfluent code.
I also added some more tests to kudu-tool-test.
Change-Id: Ie90b6228bba54575933fd9ac2f4f0bebf579ecd7
---
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_action_cluster.cc
M src/kudu/tools/tool_action_fs.cc
M src/kudu/tools/tool_action_pbc.cc
M src/kudu/tools/tool_action_tablet.cc
M src/kudu/tools/tool_main.cc
8 files changed, 261 insertions(+), 97 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/48/4148/2
--
To view, visit http://gerrit.cloudera.org:8080/4148
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie90b6228bba54575933fd9ac2f4f0bebf579ecd7
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] tool: split up action descriptions
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has submitted this change and it was merged.
Change subject: tool: split up action descriptions
......................................................................
tool: split up action descriptions
With ksck we have a use case for "short" and "long" action descriptions: the
former appears in mode help and action help, while the latter is only in
action help. This patch splits action descriptions into "short" and "extra",
refactoring all actions accordingly. While I was there I got rid of the
label abstraction, which I felt was producing too much unfluent code.
I also added some more tests to kudu-tool-test.
Change-Id: Ie90b6228bba54575933fd9ac2f4f0bebf579ecd7
Reviewed-on: http://gerrit.cloudera.org:8080/4148
Reviewed-by: Todd Lipcon <to...@apache.org>
Tested-by: Kudu Jenkins
---
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_action_cluster.cc
M src/kudu/tools/tool_action_fs.cc
M src/kudu/tools/tool_action_pbc.cc
M src/kudu/tools/tool_action_tablet.cc
M src/kudu/tools/tool_main.cc
8 files changed, 261 insertions(+), 97 deletions(-)
Approvals:
Todd Lipcon: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/4148
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie90b6228bba54575933fd9ac2f4f0bebf579ecd7
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] tool: split up action descriptions
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change.
Change subject: tool: split up action descriptions
......................................................................
Patch Set 1:
Build Started http://104.196.14.100/job/kudu-gerrit/3112/
--
To view, visit http://gerrit.cloudera.org:8080/4148
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie90b6228bba54575933fd9ac2f4f0bebf579ecd7
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] tool: split up action descriptions
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/4148
to look at the new patch set (#3).
Change subject: tool: split up action descriptions
......................................................................
tool: split up action descriptions
With ksck we have a use case for "short" and "long" action descriptions: the
former appears in mode help and action help, while the latter is only in
action help. This patch splits action descriptions into "short" and "extra",
refactoring all actions accordingly. While I was there I got rid of the
label abstraction, which I felt was producing too much unfluent code.
I also added some more tests to kudu-tool-test.
Change-Id: Ie90b6228bba54575933fd9ac2f4f0bebf579ecd7
---
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_action_cluster.cc
M src/kudu/tools/tool_action_fs.cc
M src/kudu/tools/tool_action_pbc.cc
M src/kudu/tools/tool_action_tablet.cc
M src/kudu/tools/tool_main.cc
8 files changed, 261 insertions(+), 97 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/48/4148/3
--
To view, visit http://gerrit.cloudera.org:8080/4148
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie90b6228bba54575933fd9ac2f4f0bebf579ecd7
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] tool: split up action descriptions
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change.
Change subject: tool: split up action descriptions
......................................................................
Patch Set 2:
Build Started http://104.196.14.100/job/kudu-gerrit/3113/
--
To view, visit http://gerrit.cloudera.org:8080/4148
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie90b6228bba54575933fd9ac2f4f0bebf579ecd7
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] tool: split up action descriptions
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change.
Change subject: tool: split up action descriptions
......................................................................
Patch Set 3:
Build Started http://104.196.14.100/job/kudu-gerrit/3123/
--
To view, visit http://gerrit.cloudera.org:8080/4148
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie90b6228bba54575933fd9ac2f4f0bebf579ecd7
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] tool: split up action descriptions
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: tool: split up action descriptions
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/4148
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie90b6228bba54575933fd9ac2f4f0bebf579ecd7
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] tool: split up action descriptions
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: tool: split up action descriptions
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4148/2/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:
Line 71: void RunTestAction(const string& arg_str) const {
maybe name this RunTestActionNoOutput or something so that the implied assertion is more clear?
--
To view, visit http://gerrit.cloudera.org:8080/4148
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie90b6228bba54575933fd9ac2f4f0bebf579ecd7
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] tool: split up action descriptions
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: tool: split up action descriptions
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/4148
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie90b6228bba54575933fd9ac2f4f0bebf579ecd7
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] tool: split up action descriptions
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: tool: split up action descriptions
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4148/2/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:
Line 71: void RunTestAction(const string& arg_str) const {
> maybe name this RunTestActionNoOutput or something so that the implied asse
Done
--
To view, visit http://gerrit.cloudera.org:8080/4148
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie90b6228bba54575933fd9ac2f4f0bebf579ecd7
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes