Adar Dembo has posted comments on this change. Change subject: [tools] added insert-generated-rows into kudu tools ......................................................................
Patch Set 16: (7 comments) http://gerrit.cloudera.org:8080/#/c/4412/16/src/kudu/tools/kudu-tool-test.cc File src/kudu/tools/kudu-tool-test.cc: Line 180: TEST_F(ToolTest, TestTopLevelHelp) { Can you update this test with the new mode and action? PS16, Line 781: client::sp:: Nit: you added a "using" for this at the top of the file. PS16, Line 788: vector<string>( Why is this needed? PS16, Line 789: wait(true) Can omit, this is the default behavior. PS16, Line 798: ASSERT_OK(cluster->Start()); You can move this into LoadgenCreateMiniCluster if you want. You could even move LoadgenCreateTable() (enabled via an argument) and Subprocess::Call(), then call it RunLoadgenTest() or something. Basically make the TEST_F() functions themselves just thin wrappers that provide some additional arguments. PS16, Line 819: string("-table_name=") + kTableName Nit: Substitute("-table_name=%s", kTableName) is more idiomatic. Line 820: "-buffer_flush_watermark_pct=0.125", Nit: can you use -- to prefix gflags, for consistency with existing tests? -- To view, visit http://gerrit.cloudera.org:8080/4412 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I332927c0b928c9c4fb81a8e26f5c9ed7565299ad Gerrit-PatchSet: 16 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: David Ribeiro Alves <dral...@apache.org> Gerrit-Reviewer: Dinesh Bhat <din...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes