You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Brian Wickman <wi...@apache.org> on 2014/05/13 22:03:35 UTC

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


Re: Review Request 21402: Add python checkstyle hooks.

Posted by Brian Wickman <wi...@apache.org>.

> On May 19, 2014, 7:04 p.m., Jake Farrell wrote:
> > Looks good, thoughts on saving the output to a build log file rather than /dev/null and prompting to re-run on error?

This is a good idea.  I will do this in a subsequent review though, just to get this one over the finish line.


- Brian


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21402/#review43400
-----------------------------------------------------------


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.

Posted by Jake Farrell <jf...@apache.org>.
-----------------------------------------------------------
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.

Posted by Brian Wickman <wi...@apache.org>.

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

Posted by Dan Norris <pr...@gmail.com>.
-----------------------------------------------------------
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.

Posted by Jake Farrell <jf...@apache.org>.
-----------------------------------------------------------
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.

Posted by Kevin Sweeney <ke...@apache.org>.
-----------------------------------------------------------
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.

Posted by Joe Smith <ya...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21402/#review43367
-----------------------------------------------------------

Ship it!


Ship It!

- Joe Smith


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.

Posted by Brian Wickman <wi...@apache.org>.
-----------------------------------------------------------
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