Re-enable disabled pep8 errors

[Juan Manuel Ollé]

It seems that a change introduced in this patch

https://review.openstack.org/#/c/97407/

specifically in tox.ini

[flake8]
.
.
select = H236

seems to be hidding other pep8 errors

[Juan Manuel Ollé (juan-m-olle)]

A possible fix is

select = E,W,H236

[Juan Manuel Ollé (juan-m-olle)]

select = E,W,H236 is not a valid solution, it will ignore the --ignore list of errors

[Pawel Skowron (pawel-skowron)]

I removed the select line and many errors showed up.

I am currently working on them with bug https://bugs.launchpad.net/horizon/+bug/1348610

[Pawel Skowron (pawel-skowron)]

The plan is to fix all pep8 error and then reenable the pep8 checking.

[Julie Pichon (jpichon)]

We don't necessarily want to reenable all checks, some of them are disabled on purpose (e.g. H803) and others could cause too many merge conflicts for a limited benefit.

If the current checks aren't working though that's pretty serious and should be resolved ASAP, before worrying about fixing other existing pep8 errors.

[Juan Manuel Ollé (juan-m-olle)]

Julie: Yes all errors are hide, and as Pawel said there are many error already in master.

I think, or you stop other patch be merged and fix all at once or we should start fixing and enabling in groups.

[Pawel Skowron (pawel-skowron)]

Julie:

just to be clear I believe all errors that show up after removing the line "select = H236" should be fixed.

I am NOT going to change anything around "ignore = E127,E128,H701,H702,H803", as you mentioned e.g. H803.

hope it is now more clear ;)
pawels

[Julie Pichon (jpichon)]

Pawel, yes my bad - I didn't realise things had gotten so bad we already have 2500 pep8 errors in the codebase :(

Chatting with rdopieralski and amotoki on IRC now, and although nothing is directly broken yet this still critically affects code quality, thus bumping the importance.

[Akihiro Motoki (amotoki)]

Per discussion on IRC, we agreed the following approach to enable flake8 check ASAP. (pawels/rdopieralski/jpich/amotoki)

* Remove "select" from tox.ini
* Ignore some new checks (which hits many errors)
  * Ignore H904 (Wrap long lines in parentheses instead of a backslash) is a new one and it hits over 1500.
     After fixing other errors, we discuss H904 should be enabled in Horizon later.
  * Ignore H307 (optional). Same reason as for H904.
  * If there are checks we cannot pass soon, we can ignore them temporarily and fix it later.
* Reenable flake8 check in the gate ASAP.

Error hit count per check
http://paste.openstack.org/show/88908/

[Pawel Skowron (pawel-skowron)]

Thanks for summarizing!

Let me add that we agreed also that rdopieralski will take care of "openstack_dashboard" and I will take care of "horizon" as well as tox.ini file itself, when errors are fixed.

pawels

[Akihiro Motoki (amotoki)]

Related fixes:
https://bugs.launchpad.net/horizon/+bug/1349820
https://bugs.launchpad.net/horizon/+bug/1348610

[Akihiro Motoki (amotoki)]

After introducing this bug into Horizon, hacking version used in Horizon was updated from 0.8.x to 0.9.x.
Some checks are new in hacking 0.9 and they were not checked in Horizon before.

These tests need not to be fixed in this bug and they can be fixed/discussed later.

The following is a list (not complete):

new in hacking 0.9
* H307
* H904

it seems there is a large change
* H405
   pep8 1.5.6 improves docstring checks and some rules like H405 were moved to pep8 from hacking.
   There may be a rule change.

Note that:
hacking 0.9.2 was enabled in Horizon on Jun 16.
This bug was introduced on Jun 10.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/110396

[Julie Pichon (jpichon)]

Partial patch at https://review.openstack.org/#/c/110313/

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/110313
Committed: https://git.openstack.org/cgit/openstack/horizon/commit/?id=0eca7449ccc4b1e618072352da3c79f1c0359c30
Submitter: Jenkins
Branch: master

commit 0eca7449ccc4b1e618072352da3c79f1c0359c30
Author: Radomir Dopieralski <email address hidden>
Date: Tue Jul 29 16:57:39 2014 +0200

    Fix Flake8 style warnings in openstack_dashboard/

    Warnings H904, H307 and H405 are new or considerably changed, and will
    be fixed in a separate patch.

    Closes-bug: #1349820
    Partial-bug: #1347472

    Change-Id: I4fd28990dacf16f03a4eaa6074ef59c37f1a2c14

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/110396
Committed: https://git.openstack.org/cgit/openstack/horizon/commit/?id=791f384e3e1153c26486d03c496418096ffe4b7a
Submitter: Jenkins
Branch: master

commit 791f384e3e1153c26486d03c496418096ffe4b7a
Author: Pawel Skowron <email address hidden>
Date: Tue Jul 29 20:59:41 2014 +0200

    Fix Flake8 style warnings in horizon/

    Warnings H904, H307 and H405 are new or considerably changed, and will
    be fixed in a separate patch.

    Partial-bug: #1347472

    Change-Id: Id9325a40d443f77242d3634002789501afbfc35a
    Co-Authored-By: Juan M. Olle <email address hidden>

[Akihiro Motoki (amotoki)]

I am now preparing the final change of tox.ini (and additional fix of E265 errors in doc/source/conf.py).

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/110747

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/110747
Committed: https://git.openstack.org/cgit/openstack/horizon/commit/?id=cd35fff1b6ac672ebaaa50010a6ba673c0da68a0
Submitter: Jenkins
Branch: master

commit cd35fff1b6ac672ebaaa50010a6ba673c0da68a0
Author: Akihiro Motoki <email address hidden>
Date: Thu Jul 31 02:27:02 2014 +0900

    Re-enable flake8 check

    This is the final fix to re-enable flake8/pep8 check in the gate.
    E265 errors in doc/source/conf.py are also fixed.

    Imports with "from xxxx import (xxxx)" style cannot pass "tox -e pep8"
    though they pass "run_tests.sh -p". Newlines with backslash escape
    is used in this commit to pass it.
    The reason needs to be investigated later.

    Change-Id: Ic0d765d404212d13f678b2a3aef7b9612bf5238d
    Closes-Bug: #1347472