Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/18169 )
Change subject: [tool] fix a command bug, cmd: kudu wal dump ... ...................................................................... Patch Set 2: Code-Review+1 (8 comments) Thanks for the contribution! Nice find. http://gerrit.cloudera.org:8080/#/c/18169/2/src/kudu/common/wire_protocol-test-util.h File src/kudu/common/wire_protocol-test-util.h: http://gerrit.cloudera.org:8080/#/c/18169/2/src/kudu/common/wire_protocol-test-util.h@40 PS2, Line 40: st nit: this can be const http://gerrit.cloudera.org:8080/#/c/18169/2/src/kudu/common/wire_protocol-test-util.h@41 PS2, Line 41: auto nit: const auto&? http://gerrit.cloudera.org:8080/#/c/18169/2/src/kudu/common/wire_protocol-test-util.h@41 PS2, Line 41: nit: remove two spaces here http://gerrit.cloudera.org:8080/#/c/18169/2/src/kudu/common/wire_protocol-test-util.h@58 PS2, Line 58: columns_map nit: you should be able to instantiate this inline as "...&row, { { "string_val", string(string_val) } })" http://gerrit.cloudera.org:8080/#/c/18169/2/src/kudu/common/wire_protocol-test-util.h@65 PS2, Line 65: const Schema& schema, : nit: align spaces with the ( Same below http://gerrit.cloudera.org:8080/#/c/18169/2/src/kudu/common/wire_protocol-test-util.h@77 PS2, Line 77: columns_map nit: same here http://gerrit.cloudera.org:8080/#/c/18169/2/src/kudu/tools/kudu-tool-test.cc File src/kudu/tools/kudu-tool-test.cc: http://gerrit.cloudera.org:8080/#/c/18169/2/src/kudu/tools/kudu-tool-test.cc@2119 PS2, Line 2119: Schema kSchema(GetSimpleTestSchema()); : Schema kSchemaWithIds(SchemaBuilder(kSchema).Build()); nit: since neither of these are const, we just use more standard variable names, like 'schema' and 'schema_with_ids' http://gerrit.cloudera.org:8080/#/c/18169/2/src/kudu/tools/kudu-tool-test.cc@2142 PS2, Line 2142: nit: remove a couple spaces here, and elsewhere. -- To view, visit http://gerrit.cloudera.org:8080/18169 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I27acc71597d038cafbbe687117bddb1ce16576c0 Gerrit-Change-Number: 18169 Gerrit-PatchSet: 2 Gerrit-Owner: Anonymous Coward <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Fri, 28 Jan 2022 02:02:47 +0000 Gerrit-HasComments: Yes
