You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Alexey Serbin (Code Review)" <ge...@cloudera.org> on 2017/09/02 00:06:25 UTC

[kudu-CR] [tidy] updated the 'tidy' target

Alexey Serbin has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/7947

Change subject: [tidy] updated the 'tidy' target
......................................................................

[tidy] updated the 'tidy' target

Now the 'tidy' target runs against the set of changelists since the
last committed upstream one.  This makes it consistent with how
the 'ilint' and 'iwyu' targets work.

Change-Id: I2f8db9076d5113ac44fb8200d00965f120126ac4
---
M CMakeLists.txt
M build-support/clang_tidy_gerrit.py
A build-support/tidy.sh
3 files changed, 45 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/47/7947/1
-- 
To view, visit http://gerrit.cloudera.org:8080/7947
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I2f8db9076d5113ac44fb8200d00965f120126ac4
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>

[kudu-CR] [tidy] updated the 'tidy' target

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.

Change subject: [tidy] updated the 'tidy' target
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7947/2/build-support/clang_tidy_gerrit.py
File build-support/clang_tidy_gerrit.py:

Line 169:         print >>sys.stderr, "--rev-range works only with --no-gerrit"
> Because I wanted that to work also for non-committed changes.  Specifying '
oh, and it seems the description for the '--rev-range' is outdated -- I didn't reflect the fact it picks up local changes.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2f8db9076d5113ac44fb8200d00965f120126ac4
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
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-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes

[kudu-CR] [tidy] updated the 'tidy' target

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged.

Change subject: [tidy] updated the 'tidy' target
......................................................................


[tidy] updated the 'tidy' target

Now the 'tidy' target runs against the set of changelists since the
last committed upstream one.  This makes it consistent with how
the 'ilint' and 'iwyu' targets work.

Change-Id: I2f8db9076d5113ac44fb8200d00965f120126ac4
Reviewed-on: http://gerrit.cloudera.org:8080/7947
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Kudu Jenkins
---
M CMakeLists.txt
M build-support/clang_tidy_gerrit.py
A build-support/tidy.sh
3 files changed, 45 insertions(+), 5 deletions(-)

Approvals:
  Adar Dembo: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I2f8db9076d5113ac44fb8200d00965f120126ac4
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
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] [tidy] updated the 'tidy' target

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.

Change subject: [tidy] updated the 'tidy' target
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7947/2/build-support/clang_tidy_gerrit.py
File build-support/clang_tidy_gerrit.py:

Line 169:         print >>sys.stderr, "--rev-range works only with --no-gerrit"
> I'm a little late to the party, but why not just look for ".." in the revis
Because I wanted that to work also for non-committed changes.  Specifying '..' in the revision string means it should not pick up those.

Also, overall that would not be consistent for get_gerrit_revision_url().


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2f8db9076d5113ac44fb8200d00965f120126ac4
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
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-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes

[kudu-CR] [tidy] updated the 'tidy' target

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.

Change subject: [tidy] updated the 'tidy' target
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7947/2/build-support/clang_tidy_gerrit.py
File build-support/clang_tidy_gerrit.py:

Line 169:         print >>sys.stderr, "--rev-range works only with --no-gerrit"
I'm a little late to the party, but why not just look for ".." in the revision string, and if so, use 'git diff', otherwise use 'git show'?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2f8db9076d5113ac44fb8200d00965f120126ac4
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
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-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes

[kudu-CR] [tidy] updated the 'tidy' target

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.

Change subject: [tidy] updated the 'tidy' target
......................................................................


Patch Set 1: Code-Review+2

Thanks for doing this.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2f8db9076d5113ac44fb8200d00965f120126ac4
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: No