You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Tim Armstrong (Code Review)" <ge...@cloudera.org> on 2018/07/27 15:50:33 UTC
[Impala-ASF-CR] IMPALA-7317: add scripts to post flake8 comments
Tim Armstrong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11054
Change subject: IMPALA-7317: add scripts to post flake8 comments
......................................................................
IMPALA-7317: add scripts to post flake8 comments
The script is used by a new jenkins job gerrit-auto-critic
to post comments on code reviews.
Testing:
This patch deliberately contains some flake8 violations so
that gerrit-auto-critic will flag them.
Change-Id: I7d348ea4944f829a407bd7b2939654f272736170
---
A bin/jenkins/critique-gerrit-review.py
1 file changed, 130 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/54/11054/12
--
To view, visit http://gerrit.cloudera.org:8080/11054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I7d348ea4944f829a407bd7b2939654f272736170
Gerrit-Change-Number: 11054
Gerrit-PatchSet: 12
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] IMPALA-7317: add scripts to post flake8 comments
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Hello Michael Brown, Philip Zeyliger, David Knupp, Joe McDonnell, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11054
to look at the new patch set (#15).
Change subject: IMPALA-7317: add scripts to post flake8 comments
......................................................................
IMPALA-7317: add scripts to post flake8 comments
The script is used by a new jenkins job gerrit-auto-critic
to post comments on code reviews.
Testing:
This patch deliberately contains some flake8 violations so
that gerrit-auto-critic will flag them.
Change-Id: I7d348ea4944f829a407bd7b2939654f272736170
---
A bin/jenkins/critique-gerrit-review.py
1 file changed, 131 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/54/11054/15
--
To view, visit http://gerrit.cloudera.org:8080/11054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7d348ea4944f829a407bd7b2939654f272736170
Gerrit-Change-Number: 11054
Gerrit-PatchSet: 15
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-7317: add scripts to post flake8 comments
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11054 )
Change subject: IMPALA-7317: add scripts to post flake8 comments
......................................................................
Patch Set 15:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/11054/15/bin/jenkins/critique-gerrit-review.py
File bin/jenkins/critique-gerrit-review.py:
http://gerrit.cloudera.org:8080/#/c/11054/15/bin/jenkins/critique-gerrit-review.py@50
PS15, Line 50: import time
> flake8: F401 'time' imported but unused
Done
http://gerrit.cloudera.org:8080/#/c/11054/15/bin/jenkins/critique-gerrit-review.py@108
PS15, Line 108: "
> flake8: E128 continuation line under-indented for visual indent
Done
http://gerrit.cloudera.org:8080/#/c/11054/15/bin/jenkins/critique-gerrit-review.py@127
PS15, Line 127: if __name__ == "__main__":
> flake8: E305 expected 2 blank lines after class or function definition, fou
Done
--
To view, visit http://gerrit.cloudera.org:8080/11054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7d348ea4944f829a407bd7b2939654f272736170
Gerrit-Change-Number: 11054
Gerrit-PatchSet: 15
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 30 Jul 2018 22:52:37 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7317: add scripts to post flake8 comments
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Hello Michael Brown, David Knupp, Joe McDonnell, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11054
to look at the new patch set (#13).
Change subject: IMPALA-7317: add scripts to post flake8 comments
......................................................................
IMPALA-7317: add scripts to post flake8 comments
The script is used by a new jenkins job gerrit-auto-critic
to post comments on code reviews.
Testing:
This patch deliberately contains some flake8 violations so
that gerrit-auto-critic will flag them.
Change-Id: I7d348ea4944f829a407bd7b2939654f272736170
---
A bin/jenkins/critique-gerrit-review.py
1 file changed, 129 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/54/11054/13
--
To view, visit http://gerrit.cloudera.org:8080/11054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7d348ea4944f829a407bd7b2939654f272736170
Gerrit-Change-Number: 11054
Gerrit-PatchSet: 13
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
[Impala-ASF-CR] IMPALA-7317: add scripts to post flake8 comments
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11054 )
Change subject: IMPALA-7317: add scripts to post flake8 comments
......................................................................
Patch Set 14:
Build Started https://jenkins.impala.io/job/gerrit-code-review-checks/102/
Running initial code review checks. This is experimental - please report any issues to tarmstrong@cloudera.com or on this JIRA: IMPALA-7317
--
To view, visit http://gerrit.cloudera.org:8080/11054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7d348ea4944f829a407bd7b2939654f272736170
Gerrit-Change-Number: 11054
Gerrit-PatchSet: 14
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 30 Jul 2018 17:38:33 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7317: add scripts to post flake8 comments
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11054 )
Change subject: IMPALA-7317: add scripts to post flake8 comments
......................................................................
Patch Set 17: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/11054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7d348ea4944f829a407bd7b2939654f272736170
Gerrit-Change-Number: 11054
Gerrit-PatchSet: 17
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 31 Jul 2018 02:30:53 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7317: add scripts to post flake8 comments
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Hello Michael Brown, Philip Zeyliger, David Knupp, Joe McDonnell, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11054
to look at the new patch set (#14).
Change subject: IMPALA-7317: add scripts to post flake8 comments
......................................................................
IMPALA-7317: add scripts to post flake8 comments
The script is used by a new jenkins job gerrit-auto-critic
to post comments on code reviews.
Testing:
This patch deliberately contains some flake8 violations so
that gerrit-auto-critic will flag them.
Change-Id: I7d348ea4944f829a407bd7b2939654f272736170
---
A bin/jenkins/critique-gerrit-review.py
1 file changed, 130 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/54/11054/14
--
To view, visit http://gerrit.cloudera.org:8080/11054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7d348ea4944f829a407bd7b2939654f272736170
Gerrit-Change-Number: 11054
Gerrit-PatchSet: 14
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
[Impala-ASF-CR] IMPALA-7317: add scripts to post flake8 comments
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11054 )
Change subject: IMPALA-7317: add scripts to post flake8 comments
......................................................................
Patch Set 12:
Build Started https://jenkins.impala.io/job/gerrit-code-review-checks/82/
Running initial code review checks. This is experimental - please report any issues to tarmstrong@cloudera.com or on this JIRA: IMPALA-7317
--
To view, visit http://gerrit.cloudera.org:8080/11054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7d348ea4944f829a407bd7b2939654f272736170
Gerrit-Change-Number: 11054
Gerrit-PatchSet: 12
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 27 Jul 2018 15:50:43 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7317: add scripts to post flake8 comments
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11054 )
Change subject: IMPALA-7317: add scripts to post flake8 comments
......................................................................
Patch Set 15:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/11054/15/bin/jenkins/critique-gerrit-review.py
File bin/jenkins/critique-gerrit-review.py:
http://gerrit.cloudera.org:8080/#/c/11054/15/bin/jenkins/critique-gerrit-review.py@50
PS15, Line 50: import time
flake8: F401 'time' imported but unused
http://gerrit.cloudera.org:8080/#/c/11054/15/bin/jenkins/critique-gerrit-review.py@108
PS15, Line 108: "
flake8: E128 continuation line under-indented for visual indent
http://gerrit.cloudera.org:8080/#/c/11054/15/bin/jenkins/critique-gerrit-review.py@127
PS15, Line 127: if __name__ == "__main__":
flake8: E305 expected 2 blank lines after class or function definition, found 1
--
To view, visit http://gerrit.cloudera.org:8080/11054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7d348ea4944f829a407bd7b2939654f272736170
Gerrit-Change-Number: 11054
Gerrit-PatchSet: 15
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 30 Jul 2018 22:51:20 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7317: add scripts to post flake8 comments
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Hello Michael Brown, Philip Zeyliger, David Knupp, Joe McDonnell, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11054
to look at the new patch set (#16).
Change subject: IMPALA-7317: add scripts to post flake8 comments
......................................................................
IMPALA-7317: add scripts to post flake8 comments
The script is used by a new jenkins job gerrit-auto-critic
to post comments on code reviews.
Testing:
This patch deliberately contains some flake8 violations so
that gerrit-auto-critic will flag them.
Change-Id: I7d348ea4944f829a407bd7b2939654f272736170
---
A bin/jenkins/critique-gerrit-review.py
1 file changed, 130 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/54/11054/16
--
To view, visit http://gerrit.cloudera.org:8080/11054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7d348ea4944f829a407bd7b2939654f272736170
Gerrit-Change-Number: 11054
Gerrit-PatchSet: 16
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-7317: add scripts to post flake8 comments
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11054 )
Change subject: IMPALA-7317: add scripts to post flake8 comments
......................................................................
Patch Set 15:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/105/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/11054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7d348ea4944f829a407bd7b2939654f272736170
Gerrit-Change-Number: 11054
Gerrit-PatchSet: 15
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 30 Jul 2018 23:39:22 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7317: add scripts to post flake8 comments
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11054 )
Change subject: IMPALA-7317: add scripts to post flake8 comments
......................................................................
Patch Set 16:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/11054/16/bin/jenkins/critique-gerrit-review.py
File bin/jenkins/critique-gerrit-review.py:
http://gerrit.cloudera.org:8080/#/c/11054/16/bin/jenkins/critique-gerrit-review.py@50
PS16, Line 50: FLAKE8_VERSION = "3.5.0"
flake8: F401 'time' imported but unused
http://gerrit.cloudera.org:8080/#/c/11054/16/bin/jenkins/critique-gerrit-review.py@108
PS16, Line 108: s
flake8: E128 continuation line under-indented for visual indent
http://gerrit.cloudera.org:8080/#/c/11054/16/bin/jenkins/critique-gerrit-review.py@127
PS16, Line 127: setup_virtualenv()
flake8: E305 expected 2 blank lines after class or function definition, found 1
--
To view, visit http://gerrit.cloudera.org:8080/11054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7d348ea4944f829a407bd7b2939654f272736170
Gerrit-Change-Number: 11054
Gerrit-PatchSet: 16
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 30 Jul 2018 22:52:51 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7317: add scripts to post flake8 comments
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11054 )
Change subject: IMPALA-7317: add scripts to post flake8 comments
......................................................................
Patch Set 13:
(3 comments)
Yeah I was anticipating that this might grow by adding more functions to generate comments. It's unclear if this is likely to be a useful utility library or just a script that runs on jenkins, so I figured it was easiest to keep it as a single-file script for now.
http://gerrit.cloudera.org:8080/#/c/11054/13/bin/jenkins/critique-gerrit-review.py
File bin/jenkins/critique-gerrit-review.py:
http://gerrit.cloudera.org:8080/#/c/11054/13/bin/jenkins/critique-gerrit-review.py@91
PS13, Line 91: VIOLATION_RE = re.compile(r"^([^:]*):([0-9]*):([0-9]*): (.*).*$")
> Why "(.*).*$" here? What's the second .* possibly matching, since the first
Yeah, that wasn't intended - leftover from an earlier iteration of the regex
http://gerrit.cloudera.org:8080/#/c/11054/13/bin/jenkins/critique-gerrit-review.py@102
PS13, Line 102: "start_character": col - 1, "end_character": col},
> Based on your sample here, if the character is 1, we should probably highli
Seems like a reasonable heuristic.
http://gerrit.cloudera.org:8080/#/c/11054/13/bin/jenkins/critique-gerrit-review.py@124
PS13, Line 124: comments = defaultdict(lambda: [])
: get_flake8_comments(comments)
> It's a little weird that this isn't:
I was anticipating that we might add more functions with the same API to build up a single dictionary, but it probably is still more readable in that case to return dictionaries and merge them.
--
To view, visit http://gerrit.cloudera.org:8080/11054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7d348ea4944f829a407bd7b2939654f272736170
Gerrit-Change-Number: 11054
Gerrit-PatchSet: 13
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 30 Jul 2018 17:38:25 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7317: add scripts to post flake8 comments
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11054 )
Change subject: IMPALA-7317: add scripts to post flake8 comments
......................................................................
Patch Set 14:
I ran the job pointed at a CR with no python changes: https://jenkins.impala.io/job/gerrit-auto-critic/74 and https://gerrit.cloudera.org/#/c/11007/
It produces no comments.
In order to do so I slightly extended the script so that it takes the revision to critique as an argument.
--
To view, visit http://gerrit.cloudera.org:8080/11054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7d348ea4944f829a407bd7b2939654f272736170
Gerrit-Change-Number: 11054
Gerrit-PatchSet: 14
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 30 Jul 2018 22:50:25 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7317: add scripts to post flake8 comments
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11054 )
Change subject: IMPALA-7317: add scripts to post flake8 comments
......................................................................
Patch Set 13:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/11054/13/bin/jenkins/critique-gerrit-review.py
File bin/jenkins/critique-gerrit-review.py:
http://gerrit.cloudera.org:8080/#/c/11054/13/bin/jenkins/critique-gerrit-review.py@49
PS13, Line 49: i
flake8: F401 'time' imported but unused
http://gerrit.cloudera.org:8080/#/c/11054/13/bin/jenkins/critique-gerrit-review.py@121
PS13, Line 121: i
flake8: E305 expected 2 blank lines after class or function definition, found 1
--
To view, visit http://gerrit.cloudera.org:8080/11054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7d348ea4944f829a407bd7b2939654f272736170
Gerrit-Change-Number: 11054
Gerrit-PatchSet: 13
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Comment-Date: Fri, 27 Jul 2018 15:53:51 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7317: add scripts to post flake8 comments
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11054 )
Change subject: IMPALA-7317: add scripts to post flake8 comments
......................................................................
Patch Set 12:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/82/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/11054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7d348ea4944f829a407bd7b2939654f272736170
Gerrit-Change-Number: 11054
Gerrit-PatchSet: 12
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Comment-Date: Fri, 27 Jul 2018 16:24:25 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7317: add scripts to post flake8 comments
Posted by "Michael Brown (Code Review)" <ge...@cloudera.org>.
Michael Brown has posted comments on this change. ( http://gerrit.cloudera.org:8080/11054 )
Change subject: IMPALA-7317: add scripts to post flake8 comments
......................................................................
Patch Set 17: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/11054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7d348ea4944f829a407bd7b2939654f272736170
Gerrit-Change-Number: 11054
Gerrit-PatchSet: 17
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 30 Jul 2018 22:56:32 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7317: add scripts to post flake8 comments
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11054 )
Change subject: IMPALA-7317: add scripts to post flake8 comments
......................................................................
Patch Set 15: Code-Review+2
carry
--
To view, visit http://gerrit.cloudera.org:8080/11054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7d348ea4944f829a407bd7b2939654f272736170
Gerrit-Change-Number: 11054
Gerrit-PatchSet: 15
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 30 Jul 2018 22:50:44 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7317: add scripts to post flake8 comments
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11054 )
Change subject: IMPALA-7317: add scripts to post flake8 comments
......................................................................
Patch Set 17: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/11054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7d348ea4944f829a407bd7b2939654f272736170
Gerrit-Change-Number: 11054
Gerrit-PatchSet: 17
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 30 Jul 2018 22:53:04 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7317: add scripts to post flake8 comments
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11054 )
Change subject: IMPALA-7317: add scripts to post flake8 comments
......................................................................
Patch Set 16:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/106/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/11054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7d348ea4944f829a407bd7b2939654f272736170
Gerrit-Change-Number: 11054
Gerrit-PatchSet: 16
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 31 Jul 2018 00:08:52 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7317: add scripts to post flake8 comments
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11054 )
Change subject: IMPALA-7317: add scripts to post flake8 comments
......................................................................
Patch Set 14:
Build Failed
https://jenkins.impala.io/job/gerrit-code-review-checks/102/ : Initial code review checks failed. See linked job for details on the failure.
--
To view, visit http://gerrit.cloudera.org:8080/11054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7d348ea4944f829a407bd7b2939654f272736170
Gerrit-Change-Number: 11054
Gerrit-PatchSet: 14
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 30 Jul 2018 22:37:14 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7317: add scripts to post flake8 comments
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11054 )
Change subject: IMPALA-7317: add scripts to post flake8 comments
......................................................................
IMPALA-7317: add scripts to post flake8 comments
The script is used by a new jenkins job gerrit-auto-critic
to post comments on code reviews.
Testing:
This patch deliberately contains some flake8 violations so
that gerrit-auto-critic will flag them.
Change-Id: I7d348ea4944f829a407bd7b2939654f272736170
Reviewed-on: http://gerrit.cloudera.org:8080/11054
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Reviewed-by: Michael Brown <mi...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
A bin/jenkins/critique-gerrit-review.py
1 file changed, 130 insertions(+), 0 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
Michael Brown: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/11054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I7d348ea4944f829a407bd7b2939654f272736170
Gerrit-Change-Number: 11054
Gerrit-PatchSet: 18
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-7317: add scripts to post flake8 comments
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11054 )
Change subject: IMPALA-7317: add scripts to post flake8 comments
......................................................................
Patch Set 14:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/11054/14/bin/jenkins/critique-gerrit-review.py
File bin/jenkins/critique-gerrit-review.py:
http://gerrit.cloudera.org:8080/#/c/11054/14/bin/jenkins/critique-gerrit-review.py@49
PS14, Line 49: import time
flake8: F401 'time' imported but unused
http://gerrit.cloudera.org:8080/#/c/11054/14/bin/jenkins/critique-gerrit-review.py@107
PS14, Line 107: "
flake8: E128 continuation line under-indented for visual indent
http://gerrit.cloudera.org:8080/#/c/11054/14/bin/jenkins/critique-gerrit-review.py@126
PS14, Line 126: if __name__ == "__main__":
flake8: E305 expected 2 blank lines after class or function definition, found 1
--
To view, visit http://gerrit.cloudera.org:8080/11054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7d348ea4944f829a407bd7b2939654f272736170
Gerrit-Change-Number: 11054
Gerrit-PatchSet: 14
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 30 Jul 2018 17:39:16 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7317: add scripts to post flake8 comments
Posted by "Philip Zeyliger (Code Review)" <ge...@cloudera.org>.
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/11054 )
Change subject: IMPALA-7317: add scripts to post flake8 comments
......................................................................
Patch Set 13: Code-Review+2
(3 comments)
I looked this over. I've got a few easy comments below, but I think it looks like it'll work fine.
A few comments on a bigger picture:
* Every time I write one of these things, I do exactly what you did and write a main(), and then, inevitably, on the next round I end up wrapping it in a class. You may end up doing that if you generalize the idea here to clang-tidy and so on. (Note that this reminds me of vi's ":make" feature, which takes you to the next c compiler error and has been generalized ad nauseum to the point of unusability :)
* I think we want to have an Impala python directory with a setup.py/pip-friendly structure in the near future.
* It's awkward that we're installing a virtualenv from inside this script. I think the first point here might make that unnecessary in some future. Obviously fine to do it now.
http://gerrit.cloudera.org:8080/#/c/11054/13/bin/jenkins/critique-gerrit-review.py
File bin/jenkins/critique-gerrit-review.py:
http://gerrit.cloudera.org:8080/#/c/11054/13/bin/jenkins/critique-gerrit-review.py@91
PS13, Line 91: VIOLATION_RE = re.compile(r"^([^:]*):([0-9]*):([0-9]*): (.*).*$")
Why "(.*).*$" here? What's the second .* possibly matching, since the first one is probably greedy?
http://gerrit.cloudera.org:8080/#/c/11054/13/bin/jenkins/critique-gerrit-review.py@102
PS13, Line 102: "start_character": col - 1, "end_character": col},
Based on your sample here, if the character is 1, we should probably highlight the entire line rather than just one character.
http://gerrit.cloudera.org:8080/#/c/11054/13/bin/jenkins/critique-gerrit-review.py@124
PS13, Line 124: comments = defaultdict(lambda: [])
: get_flake8_comments(comments)
It's a little weird that this isn't:
comments = get_flake8_comments()
--
To view, visit http://gerrit.cloudera.org:8080/11054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7d348ea4944f829a407bd7b2939654f272736170
Gerrit-Change-Number: 11054
Gerrit-PatchSet: 13
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Comment-Date: Mon, 30 Jul 2018 17:05:23 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7317: add scripts to post flake8 comments
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11054 )
Change subject: IMPALA-7317: add scripts to post flake8 comments
......................................................................
Patch Set 16: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/11054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7d348ea4944f829a407bd7b2939654f272736170
Gerrit-Change-Number: 11054
Gerrit-PatchSet: 16
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 30 Jul 2018 22:52:52 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7317: add scripts to post flake8 comments
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11054 )
Change subject: IMPALA-7317: add scripts to post flake8 comments
......................................................................
Patch Set 13:
Build Started https://jenkins.impala.io/job/gerrit-code-review-checks/83/
Running initial code review checks. This is experimental - please report any issues to tarmstrong@cloudera.com or on this JIRA: IMPALA-7317
--
To view, visit http://gerrit.cloudera.org:8080/11054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7d348ea4944f829a407bd7b2939654f272736170
Gerrit-Change-Number: 11054
Gerrit-PatchSet: 13
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Comment-Date: Fri, 27 Jul 2018 15:53:28 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7317: add scripts to post flake8 comments
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11054 )
Change subject: IMPALA-7317: add scripts to post flake8 comments
......................................................................
Patch Set 16:
Build Started https://jenkins.impala.io/job/gerrit-code-review-checks/106/
Running initial code review checks. This is experimental - please report any issues to tarmstrong@cloudera.com or on this JIRA: IMPALA-7317
--
To view, visit http://gerrit.cloudera.org:8080/11054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7d348ea4944f829a407bd7b2939654f272736170
Gerrit-Change-Number: 11054
Gerrit-PatchSet: 16
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 30 Jul 2018 22:52:31 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7317: add scripts to post flake8 comments
Posted by "Michael Brown (Code Review)" <ge...@cloudera.org>.
Michael Brown has posted comments on this change. ( http://gerrit.cloudera.org:8080/11054 )
Change subject: IMPALA-7317: add scripts to post flake8 comments
......................................................................
Patch Set 14:
Seems fine. Have you yet tested reviews with non-Python files that flake8 won't understand?
--
To view, visit http://gerrit.cloudera.org:8080/11054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7d348ea4944f829a407bd7b2939654f272736170
Gerrit-Change-Number: 11054
Gerrit-PatchSet: 14
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 30 Jul 2018 21:54:30 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7317: add scripts to post flake8 comments
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11054 )
Change subject: IMPALA-7317: add scripts to post flake8 comments
......................................................................
Patch Set 15:
Build Started https://jenkins.impala.io/job/gerrit-code-review-checks/105/
Running initial code review checks. This is experimental - please report any issues to tarmstrong@cloudera.com or on this JIRA: IMPALA-7317
--
To view, visit http://gerrit.cloudera.org:8080/11054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7d348ea4944f829a407bd7b2939654f272736170
Gerrit-Change-Number: 11054
Gerrit-PatchSet: 15
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 30 Jul 2018 22:50:41 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7317: add scripts to post flake8 comments
Posted by "David Knupp (Code Review)" <ge...@cloudera.org>.
David Knupp has posted comments on this change. ( http://gerrit.cloudera.org:8080/11054 )
Change subject: IMPALA-7317: add scripts to post flake8 comments
......................................................................
Patch Set 14: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/11054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7d348ea4944f829a407bd7b2939654f272736170
Gerrit-Change-Number: 11054
Gerrit-PatchSet: 14
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 30 Jul 2018 21:49:15 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7317: add scripts to post flake8 comments
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11054 )
Change subject: IMPALA-7317: add scripts to post flake8 comments
......................................................................
Patch Set 17:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2882/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/11054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7d348ea4944f829a407bd7b2939654f272736170
Gerrit-Change-Number: 11054
Gerrit-PatchSet: 17
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 30 Jul 2018 22:53:05 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7317: add scripts to post flake8 comments
Posted by "Philip Zeyliger (Code Review)" <ge...@cloudera.org>.
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/11054 )
Change subject: IMPALA-7317: add scripts to post flake8 comments
......................................................................
Patch Set 14: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/11054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7d348ea4944f829a407bd7b2939654f272736170
Gerrit-Change-Number: 11054
Gerrit-PatchSet: 14
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 30 Jul 2018 21:40:43 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7317: add scripts to post flake8 comments
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11054 )
Change subject: IMPALA-7317: add scripts to post flake8 comments
......................................................................
Patch Set 14:
Changes weren't totally trivial so I'll let you take another look. I'll fix the violations before merging too.
--
To view, visit http://gerrit.cloudera.org:8080/11054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7d348ea4944f829a407bd7b2939654f272736170
Gerrit-Change-Number: 11054
Gerrit-PatchSet: 14
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 30 Jul 2018 17:40:53 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7317: add scripts to post flake8 comments
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11054 )
Change subject: IMPALA-7317: add scripts to post flake8 comments
......................................................................
Patch Set 12:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/11054/12/bin/jenkins/critique-gerrit-review.py
File bin/jenkins/critique-gerrit-review.py:
http://gerrit.cloudera.org:8080/#/c/11054/12/bin/jenkins/critique-gerrit-review.py@49
PS12, Line 49: i
flake8: F401 'time' imported but unused
http://gerrit.cloudera.org:8080/#/c/11054/12/bin/jenkins/critique-gerrit-review.py@122
PS12, Line 122: i
flake8: E305 expected 2 blank lines after class or function definition, found 1
--
To view, visit http://gerrit.cloudera.org:8080/11054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7d348ea4944f829a407bd7b2939654f272736170
Gerrit-Change-Number: 11054
Gerrit-PatchSet: 12
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Fri, 27 Jul 2018 15:50:47 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7317: add scripts to post flake8 comments
Posted by "David Knupp (Code Review)" <ge...@cloudera.org>.
David Knupp has posted comments on this change. ( http://gerrit.cloudera.org:8080/11054 )
Change subject: IMPALA-7317: add scripts to post flake8 comments
......................................................................
Patch Set 13:
This is awesome. I look forward to reviewing this later.
--
To view, visit http://gerrit.cloudera.org:8080/11054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7d348ea4944f829a407bd7b2939654f272736170
Gerrit-Change-Number: 11054
Gerrit-PatchSet: 13
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Comment-Date: Fri, 27 Jul 2018 18:09:50 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7317: add scripts to post flake8 comments
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11054 )
Change subject: IMPALA-7317: add scripts to post flake8 comments
......................................................................
Patch Set 13:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/83/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/11054
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7d348ea4944f829a407bd7b2939654f272736170
Gerrit-Change-Number: 11054
Gerrit-PatchSet: 13
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Comment-Date: Fri, 27 Jul 2018 16:34:38 +0000
Gerrit-HasComments: No