Alexey Serbin 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? Done PS16, Line 781: client::sp:: > Nit: you added a "using" for this at the top of the file. Done PS16, Line 788: vector<string>( > Why is this needed? Because otherwise it will be an error like Bad status: Invalid argument: Table partitioning must be specified using add_hash_partitions or set_range_partition_columns PS16, Line 789: wait(true) > Can omit, this is the default behavior. Done PS16, Line 798: ASSERT_OK(cluster->Start()); > You can move this into LoadgenCreateMiniCluster if you want. You could even Good idea. For some reason, I thought keeping separate functions would be better, but apparently there isn't any demand for that. Will update. PS16, Line 819: string("-table_name=") + kTableName > Nit: Substitute("-table_name=%s", kTableName) is more idiomatic. Done Line 820: "-buffer_flush_watermark_pct=0.125", > Nit: can you use -- to prefix gflags, for consistency with existing tests? Done -- 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
