Grant Henke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13490 )

Change subject: [hms] Add a tool to list all Kudu HMS entries
......................................................................


Patch Set 2:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/13490/2/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

PS2:
> It would be great to add coverage for newly introduced commands into:
Done


http://gerrit.cloudera.org:8080/#/c/13490/2/src/kudu/tools/kudu-tool-test.cc@3887
PS2, Line 3887:   // Enable the HMS integration.
              :   cluster_->ShutdownNodes(cluster::ClusterNodes::MASTERS_ONLY);
              :   cluster_->EnableMetastoreIntegration();
> What does the tool output in case if the integration disabled?  Does it mak
Done


http://gerrit.cloudera.org:8080/#/c/13490/2/src/kudu/tools/tool_action_hms.cc
File src/kudu/tools/tool_action_hms.cc:

http://gerrit.cloudera.org:8080/#/c/13490/2/src/kudu/tools/tool_action_hms.cc@794
PS2, Line 794:   RETURN_NOT_OK(PrintHMSTables(hms_tables, cout));
             :
             :   return Status::OK();
> nit: maybe just
Done


http://gerrit.cloudera.org:8080/#/c/13490/2/src/kudu/tools/tool_action_hms.cc@857
PS2, Line 857: string(
> nit: I would expect this extra object was not needed, since Substitute() re
Done


http://gerrit.cloudera.org:8080/#/c/13490/2/src/kudu/tools/tool_action_hms.cc@859
PS2, Line 859: string(
> nit: ditto
Done


http://gerrit.cloudera.org:8080/#/c/13490/2/src/kudu/tools/tool_action_hms.cc@870
PS2, Line 870:                            .AddAction(std::move(hms_check))
             :                            .AddAction(std::move(hms_downgrade))
             :                            .AddAction(std::move(hms_fix))
             :                            .AddAction(std::move(hms_precheck))
             :                            .AddAction(std::move(hms_list))
> nit: sort alphabetically; same for the Action within 815-868 line span
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib18cde6e92e799390bb977562fd8d2eac2dae026
Gerrit-Change-Number: 13490
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Andrew Wong <[email protected]>
Gerrit-Reviewer: Grant Henke <[email protected]>
Gerrit-Reviewer: Hao Hao <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Sat, 01 Jun 2019 15:10:02 +0000
Gerrit-HasComments: Yes

Reply via email to