Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/10106 )
Change subject: Modify fix_includes.py for Kudu usage, add a wrapper ...................................................................... Patch Set 3: (10 comments) http://gerrit.cloudera.org:8080/#/c/10106/2/build-support/iwyu.py File build-support/iwyu.py: http://gerrit.cloudera.org:8080/#/c/10106/2/build-support/iwyu.py@49 PS2, Line 49: > nit: extra space Done http://gerrit.cloudera.org:8080/#/c/10106/2/build-support/iwyu.py@102 PS2, Line 102: > Isn't --name-only sufficient to yield a list of filenames? Done http://gerrit.cloudera.org:8080/#/c/10106/3/build-support/iwyu.py File build-support/iwyu.py: http://gerrit.cloudera.org:8080/#/c/10106/3/build-support/iwyu.py@137 PS3, Line 137: if '\nFATAL ERROR: ' in output or _RE_CLANG_ERROR.search(output): found that if i tried to run IWYU while there was a compilation error, it would silently not produce any results. Adding this grep fixed that. http://gerrit.cloudera.org:8080/#/c/10106/3/build-support/iwyu.py@151 PS3, Line 151: muted_paths = [p for p in paths if _is_muted(p)] oops, need to remove this and the above kwarg http://gerrit.cloudera.org:8080/#/c/10106/3/build-support/iwyu.py@165 PS3, Line 165: args = ['--quiet', '--nosafe_headers'] needed to do this or else it would never report on unused headers. http://gerrit.cloudera.org:8080/#/c/10106/3/build-support/iwyu.py@170 PS3, Line 170: args.extend(_FIX_INCLUDES_STYLE_FLAGS) : fixer_flags, _ = iwyu.fix_includes.ParseArgs(args) : oops, this is a mistaken indentation http://gerrit.cloudera.org:8080/#/c/10106/3/build-support/iwyu.py@211 PS3, Line 211: 'build.')) added this as a new feature to make it easy to fix teh whole project http://gerrit.cloudera.org:8080/#/c/10106/2/build-support/iwyu/fix_includes.py File build-support/iwyu/fix_includes.py: PS2: > FWIW, I do think it'd be useful to rework this patch into a form that'd be respectfully I don't think I have time to do this. It seems upstream there are a fair number of tests, etc, for fix_includes.py so I'd estimate that upstreaming these simple changes is likely to take a day or more. Even if I have to spend a couple hours re-applying the patch in some future release it's probably less net time to do so. http://gerrit.cloudera.org:8080/#/c/10106/2/build-support/iwyu/fix_includes.py@1507 PS2, Line 1507: """Given a file_line + file being edited, return best *_KIND value or None. > Maybe detail here how the args affect the execution of the function? Done http://gerrit.cloudera.org:8080/#/c/10106/2/build-support/iwyu/iwyu-filter.awk File build-support/iwyu/iwyu-filter.awk: http://gerrit.cloudera.org:8080/#/c/10106/2/build-support/iwyu/iwyu-filter.awk@a33 PS2, Line 33: : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : : > I would appreciate if we could keep this information elsewhere. Or, maybe, OK, I'll add a simple way to run against all files in the iwyu.py wrapper -- To view, visit http://gerrit.cloudera.org:8080/10106 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3c286271a39a0d825fb11e5610d8eb7e5b0729b9 Gerrit-Change-Number: 10106 Gerrit-PatchSet: 3 Gerrit-Owner: Todd Lipcon <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Comment-Date: Fri, 20 Apr 2018 01:13:55 +0000 Gerrit-HasComments: Yes
