> On Nov. 12, 2015, 12:05 p.m., Zameer Manji wrote:
> > Although the end result LGTM, I'm not sure if having our own custom pants 
> > plugin is the way to go here. Historically we have been very bad at 
> > upgrading pants and maintaining it, I'm afraid that if we add a custom 
> > plugin here we won't be able to upgrade pants in the future at all.

As you see fit.  I will say that the APIs used here are minimal and 
historically stable.  For example, Medium similarly enables checkstyle as well 
as another, non-default-installed plugin, `python-eval`, and they have not had 
to touch their plugin since initial install at 0.0.45 in August when it was 
released.  The other bit to know is pants is locking down to 1.0.0 as the year 
closes to isolate more radical changes.  There will be a long-term 1.0.0 branch 
the Square will be a major user of and a maintainer of with the primary focus 
being stability, secondary bugfixes, but ~no new public activity as the pants 
community charges on master towards 2.0.


- John


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


On Nov. 12, 2015, 1:54 a.m., John Sirois wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40219/
> -----------------------------------------------------------
> 
> (Updated Nov. 12, 2015, 1:54 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Joe Smith, Maxim Khutornenko, and 
> Zameer Manji.
> 
> 
> Bugs: AURORA-1532
>     https://issues.apache.org/jira/browse/AURORA-1532
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> The built-in pants python checkstyle plugin is turned on by adding a
> small custom plugin that installs the checks in the compile goal.  Now
> style checks run before compile (and thus before tests) and they benefit
> from fingerprinting; ie: if you test your changes, those tests will run
> style checks and when you go to commit, those checks will not be re-run
> by the commit hook (although files you did not test will still need to
> be checked).
> 
> A few production files were fixes up according to style failures coming
> from no space after comment opening '#', unused variables, and
> mis-aligned hanging closing parens.
> 
>  build-support/hooks/pre-commit                                               
>                                |  3 +--
>  build-support/jenkins/build.sh                                               
>                                |  9 +++++----
>  build-support/{python/checkstyle-check => plugins/3rdparty/python/BUILD}     
>                                | 13 ++-----------
>  build-support/{python/checkstyle-check => 
> plugins/src/main/python/apache/__init__.py}                       | 12 
> +-----------
>  build-support/{python/checkstyle-check => 
> plugins/src/main/python/apache/aurora/__init__.py}                | 12 
> +-----------
>  build-support/{python/checkstyle-check => 
> plugins/src/main/python/apache/aurora/pants/__init__.py}          | 12 
> +-----------
>  build-support/{python/checkstyle-check => 
> plugins/src/main/python/apache/aurora/pants/optional/BUILD}       | 18 
> +++++++-----------
>  build-support/{python/checkstyle-check => 
> plugins/src/main/python/apache/aurora/pants/optional/__init__.py} | 12 
> +-----------
>  
> build-support/plugins/src/main/python/apache/aurora/pants/optional/register.py
>                               | 36 ++++++++++++++++++++++++++++++++++++
>  build-support/python/checkstyle                                              
>                                | 34 ----------------------------------
>  build-support/python/checkstyle-check                                        
>                                |  6 +++---
>  pants.ini                                                                    
>                                | 43 
> +++++++++++++++++++++++++++++++++++++++++++
>  src/main/python/apache/aurora/admin/maintenance.py                           
>                                |  2 +-
>  src/main/python/apache/aurora/client/api/__init__.py                         
>                                |  4 ++--
>  src/main/python/apache/aurora/client/cli/client.py                           
>                                |  2 +-
>  src/main/python/apache/aurora/client/cli/cron.py                             
>                                |  2 +-
>  src/main/python/apache/thermos/core/process.py                               
>                                |  6 +++---
>  src/main/python/apache/thermos/monitoring/process_collector_psutil.py        
>                                |  1 -
>  src/test/python/apache/aurora/admin/test_admin.py                            
>                                |  6 ------
>  src/test/python/apache/aurora/admin/util.py                                  
>                                |  2 +-
>  src/test/python/apache/aurora/client/cli/test_task.py                        
>                                |  3 +--
>  21 files changed, 111 insertions(+), 127 deletions(-)
> 
> 
> Diffs
> -----
> 
>   build-support/hooks/pre-commit 619fa9e245be49e4e1f21781c0908cbf744b10ea 
>   build-support/jenkins/build.sh 5cd5242a2e4551e356e5ce5d15a0d27beb7e2d4e 
>   build-support/plugins/3rdparty/python/BUILD PRE-CREATION 
>   build-support/plugins/src/main/python/apache/__init__.py PRE-CREATION 
>   build-support/plugins/src/main/python/apache/aurora/__init__.py 
> PRE-CREATION 
>   build-support/plugins/src/main/python/apache/aurora/pants/__init__.py 
> PRE-CREATION 
>   build-support/plugins/src/main/python/apache/aurora/pants/optional/BUILD 
> PRE-CREATION 
>   
> build-support/plugins/src/main/python/apache/aurora/pants/optional/__init__.py
>  PRE-CREATION 
>   
> build-support/plugins/src/main/python/apache/aurora/pants/optional/register.py
>  PRE-CREATION 
>   build-support/python/checkstyle 61acc22613acece01580761b25afc7a3edb6b845 
>   build-support/python/checkstyle-check 
> b2bfc5dd71193a8056828e9af05a4c16965f32a1 
>   pants.ini 0bd8ec829ae65e2296a122166dea13f315323c9f 
>   src/main/python/apache/aurora/admin/maintenance.py 
> 6d94c923ae37bf6b827519d3505b100af306296b 
>   src/main/python/apache/aurora/client/api/__init__.py 
> 6f07a3073a5d422373238619d459fbd09d8adf3d 
>   src/main/python/apache/aurora/client/cli/client.py 
> 297fb588808c1eebc32ac3374265ba986dab3436 
>   src/main/python/apache/aurora/client/cli/cron.py 
> 6376fd014f2a4da29442b5c2c7eb36578b503ba3 
>   src/main/python/apache/thermos/core/process.py 
> fe95cb3be01b47616596bd78cb9a919b2e8bd978 
>   src/main/python/apache/thermos/monitoring/process_collector_psutil.py 
> f1ec5a9050ac60700c4a8afa905bcf12a9bd8a44 
>   src/test/python/apache/aurora/admin/test_admin.py 
> 8e204ab43c6bf69867ea7c32b0a7ba7fb29c0766 
>   src/test/python/apache/aurora/admin/util.py 
> 3570407b51613d0a7b4fde8a4794d88b98e150b5 
>   src/test/python/apache/aurora/client/cli/test_task.py 
> 5432a3d5f7e150b12bd75db0dac7a9018e1c6636 
> 
> Diff: https://reviews.apache.org/r/40219/diff/
> 
> 
> Testing
> -------
> 
> Ran the checks alot to get things green, but also ran into the commit hook
> failing the new plugin code itself as a serendipitous check:
> ```
> $ git commit
> Performing Python import order check.
> SUCCESS
> Performing Python checkstyle.
> 
> 
> ** PYTHON CHECKSTYLE FAILED
> *
> * For more information please run: build-support/python/checkstyle-check
> *
> **
> 
> $ build-support/python/checkstyle-check
> ...
> 00:48:50 00:01   [compile]
> 00:48:50 00:01     [compile]
> 00:48:50 00:01     [jvm]
> 00:48:50 00:01       [jvm-compilers]
> 00:48:50 00:01         [zinc-pre]
> 00:48:50 00:01         [zinc-post]
> 00:48:50 00:01     [jvm-dep-check]
> 00:48:50 00:01     [pythonstyle]
>                    Invalidated 1 target.
> F401:ERROR   
> build-support/plugins/src/main/python/apache/aurora/pants/optional/register.py:015
>  'PythonRequirement' imported but unused
>      |from pants.backend.python.python_requirement import PythonRequirement
> 
> E225:ERROR   
> build-support/plugins/src/main/python/apache/aurora/pants/optional/register.py:032
>  missing whitespace around operator
>      |  context_aware_object_factories={'pants_requirement': 
> pants_requirement_factory}
> 
> 
> FAILURE: Python Style issues found
> 
> 
> 00:48:50 00:01   [complete]
>                FAILURE
> 
> ```
> 
> 
> Thanks,
> 
> John Sirois
> 
>

Reply via email to