mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Armand Grillet <agril...@mesosphere.io>
Subject Re: Review Request 67055: Updated cpplint to be compatible with Python 3.
Date Mon, 14 May 2018 10:25:59 GMT


> On May 14, 2018, 8:53 a.m., Benjamin Bannier wrote:
> > Since the diff contains a lot of new noise, at least I don't understand why this
is needed. There should be something in the commit message explaining it.
> > 
> > As always, I'd prefer to not fork this script absent a very strong reason. I believe
the upstream script should work fine with python2 or python3, let's make any new changes to
a shared script if possible.

Thanks for the review, the updated version should solve the issues raised.


- Armand


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


On May 14, 2018, 10:22 a.m., Armand Grillet wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67055/
> -----------------------------------------------------------
> 
> (Updated May 14, 2018, 10:22 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Eric Chung, and Kevin
Klues.
> 
> 
> Bugs: MESOS-8770
>     https://issues.apache.org/jira/browse/MESOS-8770
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Uses the version distributed by Google on Apr 15, 2018.
> The diff includes the same patches as before.
> Works with Python 2 and 3.
> 
> 
> Diffs
> -----
> 
>   support/cpplint.patch 1b4d4ee62ccd40523fe9e8d6d476e1ee82e415bf 
>   support/cpplint.py 1e5d96ff2be4b9e80848e11eb0a8f4b20b31c340 
>   support/mesos-style.py 07074daa245ab503cf551201ccadeac8cc10206d 
> 
> 
> Diff: https://reviews.apache.org/r/67055/diff/3/
> 
> 
> Testing
> -------
> 
> Used `mesos-style.py`:
> ```
> apache-mesos (cpplint) $ ./support/mesos-style.py
> Checking 1289 C++ files
> Total errors found: 0
> Checking 4 JavaScript files
> Total errors found: 0
> Checking 47 Python files
>   py27-lint: commands succeeded
>   congratulations :)
>   py27-lint: commands succeeded
>   congratulations :)
> Total errors found: 0
> ```
> Updated `mesos-style.py` to use Python 3 with cpplint: 
> ```
>         process = subprocess.Popen(
>             ['python3', 'support/cpplint.py', '--quiet',
>              rules_filter] + source_paths,
>             stderr=subprocess.PIPE,
>             close_fds=True)
> ```
> 
> Run `mesos-style.py` again:
> ```
> apache-mesos (cpplint) $ ./support/mesos-style.py
> Checking 1289 C++ files
> Total errors found: 0
> Checking 4 JavaScript files
> Total errors found: 0
> Checking 47 Python files
>   py27-lint: commands succeeded
>   congratulations :)
>   py27-lint: commands succeeded
>   congratulations :)
> Total errors found: 0
> ```
> As we can see, the number of errors found by cpplint is the same as before.
> 
> I have also used cpplint separately by running `python3 support/cpplint` on a few C++
files to see that it was running as expected.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>


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