You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Attila Bukor (Code Review)" <ge...@cloudera.org> on 2019/12/05 18:18:17 UTC
[kudu-CR] Update IWYU fix includes.py
Hello Alexey Serbin, Adar Dembo, Todd Lipcon,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/14853
to review the following change.
Change subject: Update IWYU fix_includes.py
......................................................................
Update IWYU fix_includes.py
IWYU was updated to 0.13 in thirdparty along with the LLVM update to
9.0. This commit "rebases" our custom changes to fix_includes.py on top
of upstream 0.13's fix_includes.py.
Change-Id: I967382f79fc0172b79b5d42347e9d9c61ed1ea1e
---
M build-support/iwyu.py
M build-support/iwyu/fix_includes.py
2 files changed, 306 insertions(+), 105 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/53/14853/1
--
To view, visit http://gerrit.cloudera.org:8080/14853
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I967382f79fc0172b79b5d42347e9d9c61ed1ea1e
Gerrit-Change-Number: 14853
Gerrit-PatchSet: 1
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] Update IWYU fix includes.py
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14853 )
Change subject: Update IWYU fix_includes.py
......................................................................
Patch Set 2: Code-Review+2
I assume you verified it works for updated .h and .cc files when running 'make iwyu', right?
--
To view, visit http://gerrit.cloudera.org:8080/14853
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I967382f79fc0172b79b5d42347e9d9c61ed1ea1e
Gerrit-Change-Number: 14853
Gerrit-PatchSet: 2
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Thu, 05 Dec 2019 22:05:55 +0000
Gerrit-HasComments: No
[kudu-CR] Update IWYU fix includes.py
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/14853 )
Change subject: Update IWYU fix_includes.py
......................................................................
Update IWYU fix_includes.py
IWYU was updated to 0.13 in thirdparty along with the LLVM update to
9.0. This commit "rebases" our custom changes to fix_includes.py on top
of upstream 0.13's fix_includes.py.
Change-Id: I967382f79fc0172b79b5d42347e9d9c61ed1ea1e
Reviewed-on: http://gerrit.cloudera.org:8080/14853
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
M build-support/iwyu.py
M build-support/iwyu/fix_includes.py
2 files changed, 306 insertions(+), 105 deletions(-)
Approvals:
Adar Dembo: Looks good to me, approved
Kudu Jenkins: Verified
Alexey Serbin: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/14853
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I967382f79fc0172b79b5d42347e9d9c61ed1ea1e
Gerrit-Change-Number: 14853
Gerrit-PatchSet: 3
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] Update IWYU fix includes.py
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/14853 )
Change subject: Update IWYU fix_includes.py
......................................................................
Patch Set 2:
> Patch Set 2: Code-Review+2
>
> I assume you verified it works for updated .h and .cc files when running 'make iwyu', right?
Yep, works as expected as far as I can tell.
--
To view, visit http://gerrit.cloudera.org:8080/14853
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I967382f79fc0172b79b5d42347e9d9c61ed1ea1e
Gerrit-Change-Number: 14853
Gerrit-PatchSet: 2
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Thu, 05 Dec 2019 22:34:46 +0000
Gerrit-HasComments: No
[kudu-CR] Update IWYU fix includes.py
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14853 )
Change subject: Update IWYU fix_includes.py
......................................................................
Patch Set 2: Code-Review+2
Do the new IWYU goodies affect our usage of fix_includes.py? Are there any tangible changes to its recommendations?
--
To view, visit http://gerrit.cloudera.org:8080/14853
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I967382f79fc0172b79b5d42347e9d9c61ed1ea1e
Gerrit-Change-Number: 14853
Gerrit-PatchSet: 2
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Thu, 05 Dec 2019 18:42:36 +0000
Gerrit-HasComments: No
[kudu-CR] Update IWYU fix includes.py
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/14853 )
Change subject: Update IWYU fix_includes.py
......................................................................
Patch Set 2:
> Patch Set 2: Code-Review+2
>
> Do the new IWYU goodies affect our usage of fix_includes.py? Are there any tangible changes to its recommendations?
I don't think there are, there are some new flags like --reorder/--noreorder, --basedir and --only_re to match files to check with a regular expression, but I didn't change anything regarding how to invoke it.
Only reason I pushed this is because I already rebased it when we thought there was a compatibility issue with the ordering. We also might want to use these new features in the future and it should make it easier to rebase on top of newer versions if they come out.
--
To view, visit http://gerrit.cloudera.org:8080/14853
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I967382f79fc0172b79b5d42347e9d9c61ed1ea1e
Gerrit-Change-Number: 14853
Gerrit-PatchSet: 2
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Thu, 05 Dec 2019 20:42:41 +0000
Gerrit-HasComments: No