> On May 16, 2018, 7:08 p.m., Eric Chung wrote: > > > > Benjamin Bannier wrote: > I don't believe that requiring additional dependencies for these very > basic tools is a good idea, as it would add another hurdle to getting started > with contributing to Mesos (the situation is completely different for tools > configuring their own environment like e.g., the CLI where we have more > freedom). The adaptions here are also pretty minimal, so they do not > introduce a lot of additional maintenance. > > Eric Chung wrote: > there won't be any new dependencies as six is already included in pylint: > ``` > (linters) uber@echung3:~/mesos/support$ pip-compile pip-requirements.txt > No handlers could be found for logger > "pip._vendor.cachecontrol.controller" > # > # This file is autogenerated by pip-compile > # To update, run: > # > # pip-compile --output-file pip-requirements.txt pip-requirements.txt > # > --index-url > https://yoober7:ood5geishoothue8pet5iecoca9ie...@pypi.uberinternal.com/index > > astroid==1.4.9 # via pylint > backports.functools-lru-cache==1.5 # via pylint > configparser==3.5.0 # via pylint > futures==3.2.0 # via isort > isort==4.3.4 # via pylint > lazy-object-proxy==1.3.1 # via astroid > mccabe==0.6.1 # via pylint > nodeenv==1.1.2 > pylint==1.6.4 > six==1.11.0 # via astroid, pylint > wrapt==1.10.11 # via astroid > ```
`mesos-style.py` which calls `cpplint.py` is typically run from git post-commit and pre-rewrite hooks and is unrelated to anything some virtualenv might install. - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67055/#review203248 ----------------------------------------------------------- On May 16, 2018, 4:29 p.m., Armand Grillet wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67055/ > ----------------------------------------------------------- > > (Updated May 16, 2018, 4:29 p.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 > ------- > > This patch keeps the bundled cpplint to upstream '43d512b'. > Cpplint now works with Python 2 and 3 by adding part of the > content of https://github.com/google/styleguide/pull/349. > > After alias unicode to str and replace iteritems, itervalues, cpplint.py > can work well using Python3 if the linted file hasn not any issues. If > there is even only one violation of Google C++ Style Guide in the linted > file, cpplint.py will need to output error message, then exceptions like > "TypeError: write() argument must be str, not bytes" will appear. > > This is due to different supporting methods of Unicode between Python2 > and Python3. In Python2 `str` is a bytes sequence and `unicode` is used > to handle Unicode strings but in Python3 `str` is a Unicode sequence > and `unicode` does not exist. Type "bytes" is introduced to handle bytes > sequences, thus Python3 `bytes` is equal to Python2 `str`. > > The type of argument of `sys.stderr.write()` is kept the same between > Python2 and Python3 but the arguments of sys.stderr.write() are changed. > It receives a bytes sequence in Python2 and a Unicode sequence in > Python3. As a result, sys.stderr shouldn't be wrapped by a UTF-8 stream > reader and writer of codecs when using Python3. > > > Diffs > ----- > > support/cpplint.patch 69fbb5efd61962e8fef2c0188e181c789d16bae0 > support/cpplint.py 42a3dda2090a01950dcc2de7691cb3f247283e5e > > > Diff: https://reviews.apache.org/r/67055/diff/4/ > > > 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 > >