Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/8911 )
Change subject: Add 'kudu fs list' tool ...................................................................... Patch Set 3: (6 comments) http://gerrit.cloudera.org:8080/#/c/8911/2/src/kudu/tools/tool_action_fs.cc File src/kudu/tools/tool_action_fs.cc: http://gerrit.cloudera.org:8080/#/c/8911/2/src/kudu/tools/tool_action_fs.cc@580 PS2, Line 580: > Further below I got confused for a second because InfoRow returns a row to Done http://gerrit.cloudera.org:8080/#/c/8911/2/src/kudu/tools/tool_action_fs.cc@612 PS2, Line 612: > nit: missing space Done http://gerrit.cloudera.org:8080/#/c/8911/2/src/kudu/tools/tool_action_fs.cc@645 PS2, Line 645: table.AddRow(BuildInfoRow(TabletInfo, fields, tablet)); : continue; : > I think we can get rid of this and simplify a little bit since the for loop Done http://gerrit.cloudera.org:8080/#/c/8911/2/src/kudu/tools/tool_action_fs.cc@666 PS2, Line 666: AddBlockInfoRow(&ta > Would it be simpler to do great idea http://gerrit.cloudera.org:8080/#/c/8911/2/src/kudu/tools/tool_action_fs.cc@674 PS2, Line 674: for (const auto& block : > ditto (mutatis mutandis) Done http://gerrit.cloudera.org:8080/#/c/8911/2/src/kudu/tools/tool_action_fs.cc@703 PS2, Line 703: ter("fs_wal_dir") > I'd say only by setting a flag to show them I'm going to leave this TODO here, since I don't think it's crucial for a V1, but it's maybe something we'd eventually want to add. -- To view, visit http://gerrit.cloudera.org:8080/8911 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7f5a63e636d95e3ee55bb4955cece7f5d0b7532d Gerrit-Change-Number: 8911 Gerrit-PatchSet: 3 Gerrit-Owner: Dan Burkert <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Reviewer: Will Berkeley <[email protected]> Gerrit-Comment-Date: Fri, 22 Dec 2017 21:37:05 +0000 Gerrit-HasComments: Yes
