You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Mike Percy (Code Review)" <ge...@cloudera.org> on 2017/08/31 08:21:35 UTC
[kudu-CR] Add "make tidy" target
Mike Percy has uploaded a new change for review.
http://gerrit.cloudera.org:8080/7917
Change subject: Add "make tidy" target
......................................................................
Add "make tidy" target
This adds a simple cmake target to run a local clang-tidy check, instead
of having to come up with the incantation to run the tidy script.
Change-Id: I77ab0376042b7ac754a24ebc7e18bcd6dc240011
---
M CMakeLists.txt
1 file changed, 8 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/17/7917/1
--
To view, visit http://gerrit.cloudera.org:8080/7917
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I77ab0376042b7ac754a24ebc7e18bcd6dc240011
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
[kudu-CR] Add "make tidy" target
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: Add "make tidy" target
......................................................................
Patch Set 2:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/7917/2/CMakeLists.txt
File CMakeLists.txt:
Line 1117: add_custom_target(tidy ${BUILD_SUPPORT_DIR}/clang_tidy_gerrit.py -n HEAD)
Would be nice to use get-upstream-commit.sh to figure out the exact set of commits for clang-tidy to review. Then it'll have the same semantics as ilint and iwyu.
Line 1118: add_dependencies(tidy pb-gen krpc-gen)
Why does tidy depend on these?
http://gerrit.cloudera.org:8080/#/c/7917/2/README.adoc
File README.adoc:
Line 263: === Running clang-tidy checks
If you're feeling generous you could also doc the iwyu target.
--
To view, visit http://gerrit.cloudera.org:8080/7917
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I77ab0376042b7ac754a24ebc7e18bcd6dc240011
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] Add "make tidy" target
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7917
to look at the new patch set (#2).
Change subject: Add "make tidy" target
......................................................................
Add "make tidy" target
This adds a simple cmake target to run a local clang-tidy check, instead
of having to come up with the incantation to run the tidy script.
Change-Id: I77ab0376042b7ac754a24ebc7e18bcd6dc240011
---
M CMakeLists.txt
M README.adoc
2 files changed, 21 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/17/7917/2
--
To view, visit http://gerrit.cloudera.org:8080/7917
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I77ab0376042b7ac754a24ebc7e18bcd6dc240011
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] Add "make tidy" target
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has submitted this change and it was merged.
Change subject: Add "make tidy" target
......................................................................
Add "make tidy" target
This adds a simple cmake target to run a local clang-tidy check, instead
of having to come up with the incantation to run the tidy script.
Change-Id: I77ab0376042b7ac754a24ebc7e18bcd6dc240011
Reviewed-on: http://gerrit.cloudera.org:8080/7917
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
M CMakeLists.txt
M README.adoc
2 files changed, 21 insertions(+), 0 deletions(-)
Approvals:
Alexey Serbin: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/7917
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I77ab0376042b7ac754a24ebc7e18bcd6dc240011
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
[kudu-CR] Add "make tidy" target
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: Add "make tidy" target
......................................................................
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/7917/2/CMakeLists.txt
File CMakeLists.txt:
Line 1118: add_dependencies(tidy pb-gen krpc-gen)
> Why does tidy depend on these?
I think it's necessary to have those stubs and proxies generated because otherwise tidy could report on that. E.g., if one those files are not present, tidy reports something like:
INFO:root:/Users/aserbin/Projects/kudu/build-support/../src/kudu/client/client.h:30:10: error: 'kudu/client/client.pb.h' file not found [clang-diagnostic-error]
#include "kudu/client/client.pb.h"
Not sure whether those auto-generated files are needed to the analysis of the related entities in the code, though.
http://gerrit.cloudera.org:8080/#/c/7917/2/README.adoc
File README.adoc:
Line 263: === Running clang-tidy checks
> If you're feeling generous you could also doc the iwyu target.
+1
It would be nice to include that here as well. :)
--
To view, visit http://gerrit.cloudera.org:8080/7917
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I77ab0376042b7ac754a24ebc7e18bcd6dc240011
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] Add "make tidy" target
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: Add "make tidy" target
......................................................................
Patch Set 2: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/7917/2/README.adoc
File README.adoc:
Line 263: === Running clang-tidy checks
> I am feeling generous, but not that generous right now, so let's do that in
SGTM -- I will send a patch for that shortly :)
--
To view, visit http://gerrit.cloudera.org:8080/7917
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I77ab0376042b7ac754a24ebc7e18bcd6dc240011
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] Add "make tidy" target
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: Add "make tidy" target
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/7917
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I77ab0376042b7ac754a24ebc7e18bcd6dc240011
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No
[kudu-CR] Add "make tidy" target
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change.
Change subject: Add "make tidy" target
......................................................................
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/7917/2/CMakeLists.txt
File CMakeLists.txt:
Line 1117: add_custom_target(tidy ${BUILD_SUPPORT_DIR}/clang_tidy_gerrit.py -n HEAD)
> Would be nice to use get-upstream-commit.sh to figure out the exact set of
I agree it would be nice, but this is better than what we have today, and I have more pressing tasks at the moment.
http://gerrit.cloudera.org:8080/#/c/7917/2/README.adoc
File README.adoc:
Line 263: === Running clang-tidy checks
> +1
I am feeling generous, but not that generous right now, so let's do that in a follow up commit. ;)
--
To view, visit http://gerrit.cloudera.org:8080/7917
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I77ab0376042b7ac754a24ebc7e18bcd6dc240011
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] Add "make tidy" target
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: Add "make tidy" target
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/7917
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I77ab0376042b7ac754a24ebc7e18bcd6dc240011
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: No