Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/9024 )
Change subject: tools: Add debug mode to pb dump tool ...................................................................... Patch Set 1: (6 comments) http://gerrit.cloudera.org:8080/#/c/9024/1//COMMIT_MSG Commit Message: PS1: > Could you add a short test to kudu-tool-test? A run with --debug and a chec Done http://gerrit.cloudera.org:8080/#/c/9024/1//COMMIT_MSG@13 PS1, Line 13: TODO: Should this be called "verbose" mode instead of debug mode? > I don't think it really matters, since the additional information is just f Done http://gerrit.cloudera.org:8080/#/c/9024/1/src/kudu/tools/tool_action_pbc.cc File src/kudu/tools/tool_action_pbc.cc: http://gerrit.cloudera.org:8080/#/c/9024/1/src/kudu/tools/tool_action_pbc.cc@92 PS1, Line 92: } > Nit: could you add an else that does LOG(FATAL) or something like that? That's not necessary since the flags are all optional. http://gerrit.cloudera.org:8080/#/c/9024/1/src/kudu/tools/tool_action_pbc.cc@232 PS1, Line 232: .AddOptionalParameter("oneline") > Should add debug here. Done http://gerrit.cloudera.org:8080/#/c/9024/1/src/kudu/util/pb_util.cc File src/kudu/util/pb_util.cc: http://gerrit.cloudera.org:8080/#/c/9024/1/src/kudu/util/pb_util.cc@930 PS1, Line 930: // Fallthrough. > use FALLTHROUGH_INTENDED from macros.h which has some magic effect on clang removed the fallthrough http://gerrit.cloudera.org:8080/#/c/9024/1/src/kudu/util/pb_util.cc@937 PS1, Line 937: "-------" > Nit: if this is now being used in three places, perhaps make it a constant Done -- To view, visit http://gerrit.cloudera.org:8080/9024 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ica2a74a2e252d140cf7704ff68037a64db19cd80 Gerrit-Change-Number: 9024 Gerrit-PatchSet: 1 Gerrit-Owner: Mike Percy <mpe...@apache.org> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <mpe...@apache.org> Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Comment-Date: Tue, 26 Jun 2018 03:10:44 +0000 Gerrit-HasComments: Yes
