Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21496 )

Change subject: [tool] Add '--columns' param to 'table list'
......................................................................


Patch Set 8:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/21496/8//COMMIT_MSG
Commit Message:

PS8:
I missed that in earlier review iterations, but it seems there isn't any test 
coverage for the newly added flag -- that's why the typo in the 
FLAGS_colums.empty() condition wasn't caught.

It would be great to add a test for the newly added  --columns flag, similar to 
what's present for other CLI tools.  You could take a look at 
ToolTest.TestTserverList and other scenarios in kudu-tool-test.cc for reference.


http://gerrit.cloudera.org:8080/#/c/21496/7/src/kudu/tools/tool_action_table.cc
File src/kudu/tools/tool_action_table.cc:

http://gerrit.cloudera.org:8080/#/c/21496/7/src/kudu/tools/tool_action_table.cc@917
PS7, Line 917:  ListTables(const R
> Done
To be equivalent with the prior semantics, I guess it should be

if (!FLAGS_columns.empty()) {
  ...
}



--
To view, visit http://gerrit.cloudera.org:8080/21496
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I324b920e6feb6139e7d884e3cf08069b0cb922a4
Gerrit-Change-Number: 21496
Gerrit-PatchSet: 8
Gerrit-Owner: Zoltan Chovan <zcho...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <ale...@apache.org>
Gerrit-Reviewer: Anonymous Coward (763)
Gerrit-Reviewer: Attila Bukor <abu...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <greber...@gmail.com>
Gerrit-Reviewer: Zoltan Chovan <zcho...@cloudera.com>
Gerrit-Reviewer: Zoltan Martonka <zmarto...@cloudera.com>
Gerrit-Comment-Date: Tue, 18 Jun 2024 19:04:59 +0000
Gerrit-HasComments: Yes

Reply via email to