aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Brian Wickman" <wick...@apache.org>
Subject Re: Review Request 21402: Add python checkstyle hooks.
Date Thu, 15 May 2014 18:31:23 GMT


> 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message