From 4b3d6dfa8a10188995e1b027849540e1a440e88d Mon Sep 17 00:00:00 2001 From: Pilou Date: Thu, 13 Jul 2017 20:46:31 +0200 Subject: [PATCH] Use pycodestyle instead of pep8 (#25947) --- CODING_GUIDELINES.md | 10 +++++----- docs/docsite/rst/community.rst | 2 +- .../rst/dev_guide/developing_modules_documenting.rst | 2 +- docs/docsite/rst/dev_guide/testing_pep8.rst | 12 ++++++------ lib/ansible/modules/packaging/os/apt_repository.py | 1 + test/runner/lib/sanity.py | 2 +- test/runner/requirements/sanity.txt | 2 +- test/sanity/pep8/current-ignore.txt | 3 +++ 8 files changed, 19 insertions(+), 15 deletions(-) diff --git a/CODING_GUIDELINES.md b/CODING_GUIDELINES.md index bc4cee1e507..72afc7a2630 100644 --- a/CODING_GUIDELINES.md +++ b/CODING_GUIDELINES.md @@ -14,15 +14,15 @@ Language * While not all components of Ansible must be in Python, core contributions to the Ansible repo must be written in Python. This is to maximize the ability of everyone to contribute. * If you want to write non-Python ansible modules or inventory scripts, that's fine, but they are not going to get merged in most likely. Sorry!! -PEP8 and basic style checks +PEP 8 and basic style checks =========================== - * PEP8 is a great Python style guide, which you should read. - * PEP8 must not be strictly followed in all aspects, but most of it is good advice + * [PEP 8](https://www.python.org/dev/peps/pep-0008/) is a great Python style guide, which you should read. + * PEP 8 must not be strictly followed in all aspects, but most of it is good advice * In particular, we don't really care about line lengths. Buy a bigger monitor! - * To run checks for things we care about, run "make pep8" + * To run checks for things we care about, use [ansible-test](http://docs.ansible.com/ansible/dev_guide/testing_pep8.html#running-locally). * Similarly, additional checks can be made with "make pyflakes" - * There is no need to submit code changes for pep8 and pyflake fixes, as these break attribution history. Project leadership will make these periodically. + * There is no need to submit code changes for PEP 8 and pyflakes fixes, as these break attribution history. Project leadership will make these periodically. * Do not submit pull requests that simply adjust whitespace in the code Testing diff --git a/docs/docsite/rst/community.rst b/docs/docsite/rst/community.rst index 32529024565..b070ddb57bc 100644 --- a/docs/docsite/rst/community.rst +++ b/docs/docsite/rst/community.rst @@ -179,7 +179,7 @@ please observe the following guidelines: - Use a 4-space indent, not tabs. - We do not enforce 80 column lines; up to 160 columns are acceptable. - We do not accept 'style only' pull requests unless the code is nearly unreadable. -- We are "PEP8ish", but not strictly compliant. +- We are "PEP8ish", but not strictly compliant, see :doc:`testing_pep8` for more information. You can also contribute by testing and revising other requests, especially if it is one you are interested in using. Please keep your comments clear and to the point, courteous and constructive, tickets are not a diff --git a/docs/docsite/rst/dev_guide/developing_modules_documenting.rst b/docs/docsite/rst/dev_guide/developing_modules_documenting.rst index c758bf7044a..4507624fd4a 100644 --- a/docs/docsite/rst/dev_guide/developing_modules_documenting.rst +++ b/docs/docsite/rst/dev_guide/developing_modules_documenting.rst @@ -22,7 +22,7 @@ All modules must have the following sections defined in this order: .. note:: Why don't the imports go first? - Keen Python programmers may notice that contrary to PEP8's advice we don't put ``imports`` at the top of the file. This is because the ``ANSIBLE_METADATA`` through ``RETURNS`` sections are not used by the module code itself; they are essentially extra docstrings for the file. The imports are placed after these special variables for the same reason as PEP8 puts the imports after the introductory comments and docstrings. This keeps the active parts of the code together and the pieces which are purely informational apart. The decision to exclude E402 is based on readability (which is what PEP8 is about). Documentation strings in a module are much more similar to module level docstrings, than code, and are never utilized by the module itself. Placing the imports below this documentation and closer to the code, consolidates and groups all related code in a congruent manner to improve readability, debugging and understanding. + Keen Python programmers may notice that contrary to PEP 8's advice we don't put ``imports`` at the top of the file. This is because the ``ANSIBLE_METADATA`` through ``RETURNS`` sections are not used by the module code itself; they are essentially extra docstrings for the file. The imports are placed after these special variables for the same reason as PEP 8 puts the imports after the introductory comments and docstrings. This keeps the active parts of the code together and the pieces which are purely informational apart. The decision to exclude E402 is based on readability (which is what PEP 8 is about). Documentation strings in a module are much more similar to module level docstrings, than code, and are never utilized by the module itself. Placing the imports below this documentation and closer to the code, consolidates and groups all related code in a congruent manner to improve readability, debugging and understanding. .. warning:: Why do some modules have imports at the bottom of the file? diff --git a/docs/docsite/rst/dev_guide/testing_pep8.rst b/docs/docsite/rst/dev_guide/testing_pep8.rst index dd4666f1981..868751e929c 100644 --- a/docs/docsite/rst/dev_guide/testing_pep8.rst +++ b/docs/docsite/rst/dev_guide/testing_pep8.rst @@ -4,13 +4,13 @@ PEP 8 .. contents:: Topics -`PEP 8`_ style guidelines are enforced by ``pep8`` on all python files in the repository by default. +`PEP 8`_ style guidelines are enforced by `pycodestyle`_ on all python files in the repository by default. Current Rule Set ================ By default all files are tested using the current rule set. -All ``pep8`` tests are executed, except those listed in the `current ignore list`_. +All `PEP 8`_ tests are executed, except those listed in the `current ignore list`_. .. warning: Updating the Rule Set @@ -21,7 +21,7 @@ Legacy Rule Set Files which are listed in the `legacy file list`_ are tested using the legacy rule set. -All ``pep8`` tests are executed, except those listed in the `current ignore list`_ or `legacy ignore list`_. +All `PEP 8`_ tests are executed, except those listed in the `current ignore list`_ or `legacy ignore list`_. Files listed in the legacy file list which pass the current rule set will result in an error. @@ -30,7 +30,7 @@ This is intended to prevent regressions on style guidelines for files which pass Skipping Tests ============== -Files listed in the `skip list`_ are not tested by ``pep8``. +Files listed in the `skip list`_ are not tested by `pycodestyle`_. Removed Files ============= @@ -40,7 +40,7 @@ Files which have been removed from the repository must be removed from the legac Running Locally =============== -The pep8 check can be run locally with:: +The `PEP 8`_ check can be run locally with:: ./test/runner/ansible-test sanity --test pep8 [file-or-directory-path-to-check] ... @@ -48,7 +48,7 @@ The pep8 check can be run locally with:: .. _PEP 8: https://www.python.org/dev/peps/pep-0008/ -.. _pep8: https://pypi.python.org/pypi/pep8 +.. _pycodestyle: https://pypi.python.org/pypi/pycodestyle .. _current ignore list: https://github.com/ansible/ansible/blob/devel/test/sanity/pep8/current-ignore.txt .. _legacy file list: https://github.com/ansible/ansible/blob/devel/test/sanity/pep8/legacy-files.txt .. _legacy ignore list: https://github.com/ansible/ansible/blob/devel/test/sanity/pep8/legacy-ignore.txt diff --git a/lib/ansible/modules/packaging/os/apt_repository.py b/lib/ansible/modules/packaging/os/apt_repository.py index b29dd69c492..6de23bb88cc 100644 --- a/lib/ansible/modules/packaging/os/apt_repository.py +++ b/lib/ansible/modules/packaging/os/apt_repository.py @@ -210,6 +210,7 @@ class SourcesList(object): if filename is not None: return filename return '_'.join(re.sub('[^a-zA-Z0-9]', ' ', s).split()) + def _strip_username_password(s): if '@' in s: s = s.split('@', 1) diff --git a/test/runner/lib/sanity.py b/test/runner/lib/sanity.py index 04b40a38bba..22a5df62b27 100644 --- a/test/runner/lib/sanity.py +++ b/test/runner/lib/sanity.py @@ -321,7 +321,7 @@ def command_sanity_pep8(args, targets): return SanitySkipped(test) cmd = [ - 'pep8', + 'pycodestyle', '--max-line-length', '160', '--config', '/dev/null', '--ignore', ','.join(sorted(current_ignore)), diff --git a/test/runner/requirements/sanity.txt b/test/runner/requirements/sanity.txt index b87e34e189f..df8c7c0713c 100644 --- a/test/runner/requirements/sanity.txt +++ b/test/runner/requirements/sanity.txt @@ -1,8 +1,8 @@ cryptography jinja2 mock -pep8 paramiko +pycodestyle pylint pytest rstcheck diff --git a/test/sanity/pep8/current-ignore.txt b/test/sanity/pep8/current-ignore.txt index fde085633dc..a8d617457df 100644 --- a/test/sanity/pep8/current-ignore.txt +++ b/test/sanity/pep8/current-ignore.txt @@ -1 +1,4 @@ +E305 E402 +E722 +E741