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