You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Todd Lipcon (Code Review)" <ge...@cloudera.org> on 2016/12/22 15:49:18 UTC
[kudu-CR] KUDU-1812. Clang tool for replacing DebugString calls
Hello Dan Burkert, Adar Dembo,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/5560
to review the following change.
Change subject: KUDU-1812. Clang tool for replacing DebugString calls
......................................................................
KUDU-1812. Clang tool for replacing DebugString calls
This is a clang-based tool that auto-replaces pb.DebugString or
pb.ShortDebugString call with SecureDebugString(pb) or
SecureShortDebugString(pb).
It's not 100% effective due to some complexity with macros that I
couldn't figure out, but seemed to cover 99% of the cases, and I managed
to get it to insert TODOs in most of the spots it couldn't figure out.
Manual grepping can take care of the remaining spots.
This can be run using:
$ cd build/latest
$ cmake -DCMAKE_EXPORT_COMPILE_COMMANDS=1 ../../
$ make
$ find ../../src/kudu/ -name \*.cc | \
xargs -P8 -n1 ../../build-support/tools/kudu-lint/run-pb-refactor.sh
$ clang-apply-replacements /tmp/repl/
It doesn't take care of auto-adding necessary includes, but was in
general useful.
The usefulness of this tool is probably fleeting -- we could decide to
not commit this patch at all, or we could commit it and later repurpose
it (in a simpler form) as a pre-commit check that no new calls to
DebugString are added.
Change-Id: If74fc3cc69c3d6048abaa3b0d1f1ff2b2aaeec00
---
M build-support/tools/kudu-lint/CMakeLists.txt
A build-support/tools/kudu-lint/refactor-pb-tostring.cc
A build-support/tools/kudu-lint/run-pb-refactor.sh
3 files changed, 201 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/60/5560/1
--
To view, visit http://gerrit.cloudera.org:8080/5560
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: If74fc3cc69c3d6048abaa3b0d1f1ff2b2aaeec00
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
[kudu-CR] KUDU-1812. Clang tool for replacing DebugString calls
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has abandoned this change. ( http://gerrit.cloudera.org:8080/5560 )
Change subject: KUDU-1812. Clang tool for replacing DebugString calls
......................................................................
Abandoned
This hasn't proven particularly necessary.
--
To view, visit http://gerrit.cloudera.org:8080/5560
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: If74fc3cc69c3d6048abaa3b0d1f1ff2b2aaeec00
Gerrit-Change-Number: 5560
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-1812. Clang tool for replacing DebugString calls
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: KUDU-1812. Clang tool for replacing DebugString calls
......................................................................
Patch Set 1:
(2 comments)
I don't have a strong opinion on whether to merge this now or later. If we're going to merge it now, I suppose it'd be nice to clean up run-pb-refactor.sh a bit to make more what's going on more clear. But we could just as easily wait.
http://gerrit.cloudera.org:8080/#/c/5560/1//COMMIT_MSG
Commit Message:
PS1, Line 23: 8
Probably $(nproc), right?
PS1, Line 30: or we could commit it and later repurpose
: it (in a simpler form) as a pre-commit check that no new calls to
: DebugString are added.
+1 to this
--
To view, visit http://gerrit.cloudera.org:8080/5560
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If74fc3cc69c3d6048abaa3b0d1f1ff2b2aaeec00
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes