Adar Dembo has posted comments on this change.

Change subject: [build-support] added IWYU filter script
......................................................................


Patch Set 2:

(12 comments)

http://gerrit.cloudera.org:8080/#/c/7604/2/build-support/iwyu/iwyu-filter.awk
File build-support/iwyu/iwyu-filter.awk:

PS2, Line 19: include-what-you-use
"include-what-you-use (IWYU) tool"

Then you can use "IWYU" henceforth in the file instead of having to repeat the 
more cumbersome "include-what-you-use".


PS2, Line 20: for
of


PS2, Line 21: amended
Is "amended" the right word here? The pragmas silences certain IWYU 
recommendations, no?


PS2, Line 24: # Also, it's possible to address the boost-related headers using 
special
            : # mapping (the mapping needs to be implemented).
Can you provide an example of what this is referring to?


PS2, Line 29: For this, CMake version of 3.3 and higher is required.
Doesn't Kudu require a higher version anyway? So maybe this recommendation 
doesn't really matter?


Line 34: #     
IWYU=<abs_path_kudu>/thirdparty/clang-toolchain/bin/include-what-you-use
Why does this have to be an absolute path? You could make it absolute using 
`pwd`/../../thirdparty/..., right?


Line 39: #       -DCMAKE_BUILD_TYPE=debug \
Isn't this the default value? Can be omitted?


PS2, Line 47: <desired_number_of_parallel_jobs>
Just replace with $(nproc)


PS2, Line 56: to
drop this


PS2, Line 56: the
and this


Line 645:   in_ctx = 1
I think I understand what in_ctx does; what does this do?


PS2, Line 648: ~
Isn't this a regex match? Why not a string comparison?


-- 
To view, visit http://gerrit.cloudera.org:8080/7604
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idae8dae3e488151590d5420adc1f0a084339e2fa
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <aser...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <danburk...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-HasComments: Yes

Reply via email to