Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/19501 )
Change subject: [Tool] Show the information of a tablet ...................................................................... Patch Set 3: (6 comments) http://gerrit.cloudera.org:8080/#/c/19501/3/src/kudu/client/tablet-internal.h File src/kudu/client/tablet-internal.h: http://gerrit.cloudera.org:8080/#/c/19501/3/src/kudu/client/tablet-internal.h@31 PS3, Line 31: Data The variables' initialize order in construct function show be same as the order happens in class ? http://gerrit.cloudera.org:8080/#/c/19501/3/src/kudu/master/master.proto File src/kudu/master/master.proto: http://gerrit.cloudera.org:8080/#/c/19501/3/src/kudu/master/master.proto@448 PS3, Line 448: optional bytes table_id = 8; : optional string table_name = 9; Whether is it necessary to move them to the end? http://gerrit.cloudera.org:8080/#/c/19501/3/src/kudu/tools/kudu-tool-test.cc File src/kudu/tools/kudu-tool-test.cc: http://gerrit.cloudera.org:8080/#/c/19501/3/src/kudu/tools/kudu-tool-test.cc@3996 PS3, Line 3996: ASSERT_GE This assert seems useless? http://gerrit.cloudera.org:8080/#/c/19501/3/src/kudu/tools/tool_action_tablet.cc File src/kudu/tools/tool_action_tablet.cc: http://gerrit.cloudera.org:8080/#/c/19501/3/src/kudu/tools/tool_action_tablet.cc@295 PS3, Line 295: FLAGS_format 'FLAGS_format' has many values as below. 125 DEFINE_string(format, "pretty", 126 "Format to use for printing list output tables.\n" 127 "Possible values: pretty, space, tsv, csv, and json"); If we only focus on 'json', I think the gflag 'FLAGS_json' as below may be better. 'src/kudu/tools/tool_action_pbc.cc:70:DEFINE_bool(json, false, "Print protobufs in JSON format");' http://gerrit.cloudera.org:8080/#/c/19501/3/src/kudu/tools/tool_action_tablet.cc@321 PS3, Line 321: nit: remote an empty line http://gerrit.cloudera.org:8080/#/c/19501/3/src/kudu/tools/tool_action_tablet.cc@409 PS3, Line 409: show_info what about 'describe' ? -- To view, visit http://gerrit.cloudera.org:8080/19501 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib5ae5f61f50a44c4787843df76adaa61700ae9fe Gerrit-Change-Number: 19501 Gerrit-PatchSet: 3 Gerrit-Owner: Wang Xixu <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Wang Xixu <[email protected]> Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Reviewer: Yuqi Du <[email protected]> Gerrit-Comment-Date: Fri, 24 Feb 2023 11:41:11 +0000 Gerrit-HasComments: Yes
