Re: Review Request 21402: Add python checkstyle hooks.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21402/#review46638
---
Ship it!
Looks good, please create a ticket to capture the log output so it is not
forgotten about
- Jake Farrell
On May 13, 2014, 8:04 p.m., Brian Wickman wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21402/
---
(Updated May 13, 2014, 8:04 p.m.)
Review request for Aurora, Jake Farrell and Kevin Sweeney.
Bugs: AURORA-149
https://issues.apache.org/jira/browse/AURORA-149
Repository: aurora
Description
---
Consolidates python checks into build-support/python.
Adds checkstyle tool, which runs pep8, pyflakes and a number of general code
cleanliness plugins.
Should I add the proposed style guide in this review or is a subsequent
review fine? It's essentially 2sp indent, 100-col lines, naming conventions
+ a few things to make life easy in a 2.x+3.x codebase. These latter plugins
can be omitted via the -n option in the checkstyle tool.
I have a separate branch that goes through and fixes all the checkstyle
errors. It's a ~1000 line diff but mostly minor changes.
Diffs
-
.gitignore cd7bc6dd122fd3490568fe5b2a68d59cfff7fbff
build-support/hooks/pre-commit 21e2b060bc6293d259f8da1d8ac75a214b74ac1a
build-support/isort a5378f1e976c043b23cde62ec5c2581757e35d4f
build-support/isort-check 4129deb322a1cf7dba32eeec31238cf70648356a
build-support/isort-run c528b9a45e0b9281e0e295b55b3cd762b14c8b07
build-support/python/checkstyle PRE-CREATION
build-support/python/checkstyle-check PRE-CREATION
src/main/python/apache/aurora/client/cli/task.py
1bd746da2f39f78b1497701f69777ca3ea6b70ea
Diff: https://reviews.apache.org/r/21402/diff/
Testing
---
mba=aurora=; build-support/python/checkstyle-check src/main/python/ | grep
^\w | awk '{ print $1 }' | sort | uniq -c | sort -rn
120 E251:ERROR
90 F401:ERROR
86 T001:ERROR
65 E221:ERROR
28 T800:WARNING
27 T301:ERROR
22 T302:ERROR
20 T607:ERROR
19 E261:ERROR
17 E303:ERROR
14 E501:ERROR
13 F841:ERROR
11 E226:ERROR
8 F821:ERROR
6 T803:ERROR
6 T802:WARNING
4 T000:ERROR
4 F403:ERROR
4 E712:ERROR
3 E241:ERROR
3 E203:ERROR
3 E202:ERROR
2 T801:ERROR
2 T602:ERROR
2 T200:ERROR
2 T100:ERROR
2 T002:ERROR
2 E711:ERROR
2 E201:ERROR
1 E231:ERROR
1 E222:ERROR
1 E122:ERROR
Thanks,
Brian Wickman
Re: Review Request 21402: Add python checkstyle hooks.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21402/#review43389
---
Ship it!
Ship It!
- Kevin Sweeney
On May 13, 2014, 1:04 p.m., Brian Wickman wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21402/
---
(Updated May 13, 2014, 1:04 p.m.)
Review request for Aurora, Jake Farrell and Kevin Sweeney.
Bugs: AURORA-149
https://issues.apache.org/jira/browse/AURORA-149
Repository: aurora
Description
---
Consolidates python checks into build-support/python.
Adds checkstyle tool, which runs pep8, pyflakes and a number of general code
cleanliness plugins.
Should I add the proposed style guide in this review or is a subsequent
review fine? It's essentially 2sp indent, 100-col lines, naming conventions
+ a few things to make life easy in a 2.x+3.x codebase. These latter plugins
can be omitted via the -n option in the checkstyle tool.
I have a separate branch that goes through and fixes all the checkstyle
errors. It's a ~1000 line diff but mostly minor changes.
Diffs
-
.gitignore cd7bc6dd122fd3490568fe5b2a68d59cfff7fbff
build-support/hooks/pre-commit 21e2b060bc6293d259f8da1d8ac75a214b74ac1a
build-support/isort a5378f1e976c043b23cde62ec5c2581757e35d4f
build-support/isort-check 4129deb322a1cf7dba32eeec31238cf70648356a
build-support/isort-run c528b9a45e0b9281e0e295b55b3cd762b14c8b07
build-support/python/checkstyle PRE-CREATION
build-support/python/checkstyle-check PRE-CREATION
src/main/python/apache/aurora/client/cli/task.py
1bd746da2f39f78b1497701f69777ca3ea6b70ea
Diff: https://reviews.apache.org/r/21402/diff/
Testing
---
mba=aurora=; build-support/python/checkstyle-check src/main/python/ | grep
^\w | awk '{ print $1 }' | sort | uniq -c | sort -rn
120 E251:ERROR
90 F401:ERROR
86 T001:ERROR
65 E221:ERROR
28 T800:WARNING
27 T301:ERROR
22 T302:ERROR
20 T607:ERROR
19 E261:ERROR
17 E303:ERROR
14 E501:ERROR
13 F841:ERROR
11 E226:ERROR
8 F821:ERROR
6 T803:ERROR
6 T802:WARNING
4 T000:ERROR
4 F403:ERROR
4 E712:ERROR
3 E241:ERROR
3 E203:ERROR
3 E202:ERROR
2 T801:ERROR
2 T602:ERROR
2 T200:ERROR
2 T100:ERROR
2 T002:ERROR
2 E711:ERROR
2 E201:ERROR
1 E231:ERROR
1 E222:ERROR
1 E122:ERROR
Thanks,
Brian Wickman
Re: Review Request 21402: Add python checkstyle hooks.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21402/#review43400
---
Ship it!
Looks good, thoughts on saving the output to a build log file rather than
/dev/null and prompting to re-run on error?
- Jake Farrell
On May 13, 2014, 8:04 p.m., Brian Wickman wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21402/
---
(Updated May 13, 2014, 8:04 p.m.)
Review request for Aurora, Jake Farrell and Kevin Sweeney.
Bugs: AURORA-149
https://issues.apache.org/jira/browse/AURORA-149
Repository: aurora
Description
---
Consolidates python checks into build-support/python.
Adds checkstyle tool, which runs pep8, pyflakes and a number of general code
cleanliness plugins.
Should I add the proposed style guide in this review or is a subsequent
review fine? It's essentially 2sp indent, 100-col lines, naming conventions
+ a few things to make life easy in a 2.x+3.x codebase. These latter plugins
can be omitted via the -n option in the checkstyle tool.
I have a separate branch that goes through and fixes all the checkstyle
errors. It's a ~1000 line diff but mostly minor changes.
Diffs
-
.gitignore cd7bc6dd122fd3490568fe5b2a68d59cfff7fbff
build-support/hooks/pre-commit 21e2b060bc6293d259f8da1d8ac75a214b74ac1a
build-support/isort a5378f1e976c043b23cde62ec5c2581757e35d4f
build-support/isort-check 4129deb322a1cf7dba32eeec31238cf70648356a
build-support/isort-run c528b9a45e0b9281e0e295b55b3cd762b14c8b07
build-support/python/checkstyle PRE-CREATION
build-support/python/checkstyle-check PRE-CREATION
src/main/python/apache/aurora/client/cli/task.py
1bd746da2f39f78b1497701f69777ca3ea6b70ea
Diff: https://reviews.apache.org/r/21402/diff/
Testing
---
mba=aurora=; build-support/python/checkstyle-check src/main/python/ | grep
^\w | awk '{ print $1 }' | sort | uniq -c | sort -rn
120 E251:ERROR
90 F401:ERROR
86 T001:ERROR
65 E221:ERROR
28 T800:WARNING
27 T301:ERROR
22 T302:ERROR
20 T607:ERROR
19 E261:ERROR
17 E303:ERROR
14 E501:ERROR
13 F841:ERROR
11 E226:ERROR
8 F821:ERROR
6 T803:ERROR
6 T802:WARNING
4 T000:ERROR
4 F403:ERROR
4 E712:ERROR
3 E241:ERROR
3 E203:ERROR
3 E202:ERROR
2 T801:ERROR
2 T602:ERROR
2 T200:ERROR
2 T100:ERROR
2 T002:ERROR
2 E711:ERROR
2 E201:ERROR
1 E231:ERROR
1 E222:ERROR
1 E122:ERROR
Thanks,
Brian Wickman
Re: Review Request 21402: Add python checkstyle hooks.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21402/#review43078
---
build-support/python/checkstyle-check
https://reviews.apache.org/r/21402/#comment77101
Could you consolidate checkstyle and checkstyle check by sourcing the venv
and calling deactivate once you're done?
- Dan Norris
On May 13, 2014, 8:04 p.m., Brian Wickman wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21402/
---
(Updated May 13, 2014, 8:04 p.m.)
Review request for Aurora, Jake Farrell and Kevin Sweeney.
Bugs: AURORA-149
https://issues.apache.org/jira/browse/AURORA-149
Repository: aurora
Description
---
Consolidates python checks into build-support/python.
Adds checkstyle tool, which runs pep8, pyflakes and a number of general code
cleanliness plugins.
Should I add the proposed style guide in this review or is a subsequent
review fine? It's essentially 2sp indent, 100-col lines, naming conventions
+ a few things to make life easy in a 2.x+3.x codebase. These latter plugins
can be omitted via the -n option in the checkstyle tool.
I have a separate branch that goes through and fixes all the checkstyle
errors. It's a ~1000 line diff but mostly minor changes.
Diffs
-
.gitignore cd7bc6dd122fd3490568fe5b2a68d59cfff7fbff
build-support/hooks/pre-commit 21e2b060bc6293d259f8da1d8ac75a214b74ac1a
build-support/isort a5378f1e976c043b23cde62ec5c2581757e35d4f
build-support/isort-check 4129deb322a1cf7dba32eeec31238cf70648356a
build-support/isort-run c528b9a45e0b9281e0e295b55b3cd762b14c8b07
build-support/python/checkstyle PRE-CREATION
build-support/python/checkstyle-check PRE-CREATION
src/main/python/apache/aurora/client/cli/task.py
1bd746da2f39f78b1497701f69777ca3ea6b70ea
Diff: https://reviews.apache.org/r/21402/diff/
Testing
---
mba=aurora=; build-support/python/checkstyle-check src/main/python/ | grep
^\w | awk '{ print $1 }' | sort | uniq -c | sort -rn
120 E251:ERROR
90 F401:ERROR
86 T001:ERROR
65 E221:ERROR
28 T800:WARNING
27 T301:ERROR
22 T302:ERROR
20 T607:ERROR
19 E261:ERROR
17 E303:ERROR
14 E501:ERROR
13 F841:ERROR
11 E226:ERROR
8 F821:ERROR
6 T803:ERROR
6 T802:WARNING
4 T000:ERROR
4 F403:ERROR
4 E712:ERROR
3 E241:ERROR
3 E203:ERROR
3 E202:ERROR
2 T801:ERROR
2 T602:ERROR
2 T200:ERROR
2 T100:ERROR
2 T002:ERROR
2 E711:ERROR
2 E201:ERROR
1 E231:ERROR
1 E222:ERROR
1 E122:ERROR
Thanks,
Brian Wickman
Re: Review Request 21402: Add python checkstyle hooks.
On May 15, 2014, 2:01 a.m., Dan Norris wrote:
build-support/python/checkstyle-check, line 23
https://reviews.apache.org/r/21402/diff/2/?file=580886#file580886line23
Could you consolidate checkstyle and checkstyle check by sourcing the
venv and calling deactivate once you're done?
The rationalization for separating them is that checkstyle is just responsible
for running the command, whereas checkstyle-check runs the command in the
fashion that should gate commits. This means that if you want to just run a
particular checkstyle against a particular directory and not diffed against
master (e.g. build-support/python/checkstyle -p PyflakesPlugin
src/main/python/apache/aurora/executor) that's still possible.
This is also the same as what we did for isort by splitting into isort,
isort-check and isort-run.
- Brian
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21402/#review43078
---
On May 13, 2014, 8:04 p.m., Brian Wickman wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21402/
---
(Updated May 13, 2014, 8:04 p.m.)
Review request for Aurora, Jake Farrell and Kevin Sweeney.
Bugs: AURORA-149
https://issues.apache.org/jira/browse/AURORA-149
Repository: aurora
Description
---
Consolidates python checks into build-support/python.
Adds checkstyle tool, which runs pep8, pyflakes and a number of general code
cleanliness plugins.
Should I add the proposed style guide in this review or is a subsequent
review fine? It's essentially 2sp indent, 100-col lines, naming conventions
+ a few things to make life easy in a 2.x+3.x codebase. These latter plugins
can be omitted via the -n option in the checkstyle tool.
I have a separate branch that goes through and fixes all the checkstyle
errors. It's a ~1000 line diff but mostly minor changes.
Diffs
-
.gitignore cd7bc6dd122fd3490568fe5b2a68d59cfff7fbff
build-support/hooks/pre-commit 21e2b060bc6293d259f8da1d8ac75a214b74ac1a
build-support/isort a5378f1e976c043b23cde62ec5c2581757e35d4f
build-support/isort-check 4129deb322a1cf7dba32eeec31238cf70648356a
build-support/isort-run c528b9a45e0b9281e0e295b55b3cd762b14c8b07
build-support/python/checkstyle PRE-CREATION
build-support/python/checkstyle-check PRE-CREATION
src/main/python/apache/aurora/client/cli/task.py
1bd746da2f39f78b1497701f69777ca3ea6b70ea
Diff: https://reviews.apache.org/r/21402/diff/
Testing
---
mba=aurora=; build-support/python/checkstyle-check src/main/python/ | grep
^\w | awk '{ print $1 }' | sort | uniq -c | sort -rn
120 E251:ERROR
90 F401:ERROR
86 T001:ERROR
65 E221:ERROR
28 T800:WARNING
27 T301:ERROR
22 T302:ERROR
20 T607:ERROR
19 E261:ERROR
17 E303:ERROR
14 E501:ERROR
13 F841:ERROR
11 E226:ERROR
8 F821:ERROR
6 T803:ERROR
6 T802:WARNING
4 T000:ERROR
4 F403:ERROR
4 E712:ERROR
3 E241:ERROR
3 E203:ERROR
3 E202:ERROR
2 T801:ERROR
2 T602:ERROR
2 T200:ERROR
2 T100:ERROR
2 T002:ERROR
2 E711:ERROR
2 E201:ERROR
1 E231:ERROR
1 E222:ERROR
1 E122:ERROR
Thanks,
Brian Wickman
Re: Review Request 21402: Add python checkstyle hooks.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21402/
---
(Updated May 13, 2014, 8:04 p.m.)
Review request for Aurora, Jake Farrell and Kevin Sweeney.
Changes
---
Add $@ to checkstyle-check so that default (--diff=master) can be overridden
with source paths.
Bugs: AURORA-149
https://issues.apache.org/jira/browse/AURORA-149
Repository: aurora
Description
---
Consolidates python checks into build-support/python.
Adds checkstyle tool, which runs pep8, pyflakes and a number of general code
cleanliness plugins.
Should I add the proposed style guide in this review or is a subsequent review
fine? It's essentially 2sp indent, 100-col lines, naming conventions + a few
things to make life easy in a 2.x+3.x codebase. These latter plugins can be
omitted via the -n option in the checkstyle tool.
I have a separate branch that goes through and fixes all the checkstyle errors.
It's a ~1000 line diff but mostly minor changes.
Diffs (updated)
-
.gitignore cd7bc6dd122fd3490568fe5b2a68d59cfff7fbff
build-support/hooks/pre-commit 21e2b060bc6293d259f8da1d8ac75a214b74ac1a
build-support/isort a5378f1e976c043b23cde62ec5c2581757e35d4f
build-support/isort-check 4129deb322a1cf7dba32eeec31238cf70648356a
build-support/isort-run c528b9a45e0b9281e0e295b55b3cd762b14c8b07
build-support/python/checkstyle PRE-CREATION
build-support/python/checkstyle-check PRE-CREATION
src/main/python/apache/aurora/client/cli/task.py
1bd746da2f39f78b1497701f69777ca3ea6b70ea
Diff: https://reviews.apache.org/r/21402/diff/
Testing
---
mba=aurora=; build-support/python/checkstyle-check src/main/python/ | grep
^\w | awk '{ print $1 }' | sort | uniq -c | sort -rn
120 E251:ERROR
90 F401:ERROR
86 T001:ERROR
65 E221:ERROR
28 T800:WARNING
27 T301:ERROR
22 T302:ERROR
20 T607:ERROR
19 E261:ERROR
17 E303:ERROR
14 E501:ERROR
13 F841:ERROR
11 E226:ERROR
8 F821:ERROR
6 T803:ERROR
6 T802:WARNING
4 T000:ERROR
4 F403:ERROR
4 E712:ERROR
3 E241:ERROR
3 E203:ERROR
3 E202:ERROR
2 T801:ERROR
2 T602:ERROR
2 T200:ERROR
2 T100:ERROR
2 T002:ERROR
2 E711:ERROR
2 E201:ERROR
1 E231:ERROR
1 E222:ERROR
1 E122:ERROR
Thanks,
Brian Wickman
Review Request 21402: Add python checkstyle hooks.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21402/
---
Review request for Aurora, Jake Farrell and Kevin Sweeney.
Bugs: AURORA-149
https://issues.apache.org/jira/browse/AURORA-149
Repository: aurora
Description
---
Consolidates python checks into build-support/python.
Adds checkstyle tool, which runs pep8, pyflakes and a number of general code
cleanliness plugins.
Should I add the proposed style guide in this review or is a subsequent review
fine? It's essentially 2sp indent, 100-col lines, naming conventions + a few
things to make life easy in a 2.x+3.x codebase. These latter plugins can be
omitted via the -n option in the checkstyle tool.
I have a separate branch that goes through and fixes all the checkstyle errors.
It's a ~1000 line diff but mostly minor changes.
Diffs
-
.gitignore cd7bc6dd122fd3490568fe5b2a68d59cfff7fbff
build-support/hooks/pre-commit 21e2b060bc6293d259f8da1d8ac75a214b74ac1a
build-support/isort a5378f1e976c043b23cde62ec5c2581757e35d4f
build-support/isort-check 4129deb322a1cf7dba32eeec31238cf70648356a
build-support/isort-run c528b9a45e0b9281e0e295b55b3cd762b14c8b07
build-support/python/checkstyle PRE-CREATION
build-support/python/checkstyle-check PRE-CREATION
src/main/python/apache/aurora/client/cli/task.py
1bd746da2f39f78b1497701f69777ca3ea6b70ea
Diff: https://reviews.apache.org/r/21402/diff/
Testing
---
mba=aurora=; build-support/python/checkstyle-check src/main/python/ | grep
^\w | awk '{ print $1 }' | sort | uniq -c | sort -rn
120 E251:ERROR
90 F401:ERROR
86 T001:ERROR
65 E221:ERROR
28 T800:WARNING
27 T301:ERROR
22 T302:ERROR
20 T607:ERROR
19 E261:ERROR
17 E303:ERROR
14 E501:ERROR
13 F841:ERROR
11 E226:ERROR
8 F821:ERROR
6 T803:ERROR
6 T802:WARNING
4 T000:ERROR
4 F403:ERROR
4 E712:ERROR
3 E241:ERROR
3 E203:ERROR
3 E202:ERROR
2 T801:ERROR
2 T602:ERROR
2 T200:ERROR
2 T100:ERROR
2 T002:ERROR
2 E711:ERROR
2 E201:ERROR
1 E231:ERROR
1 E222:ERROR
1 E122:ERROR
Thanks,
Brian Wickman
