Re: [PATCH 1/2] build: Add flak8 env

[Nir Soffer]

On Sun, Jun 16, 2019 at 3:32 PM Pavel Bar  wrote:
> On Fri, Jun 14, 2019 at 8:15 PM Nir Soffer  wrote:
> > Flake8 provides basic linting and style checks for python code, helping
> > to keep the source in good shape.
> >
> > Running tox will run flake8 automatically, showing errors in the source
> > and statistics for current issues. To run only flake8 tests use:
> > $ tox -e flake8
> >
> > We explicitly ignore errors about star import (from . units import *),
> > since these imports make the source much more readable.
> >
> > Running tox will fail now with these errors:
> >
> > 1     E203 whitespace before ','
> > 1     E265 block comment should start with '# '
> > 7     E302 expected 2 blank lines, found 1
> > 1     E305 expected 2 blank lines after class or function definition,
> > found 1
> > 14    E501 line too long (82 > 79 characters)
>
> What does it mean? What's the line limit? 79?

flake8 default max-line-length is 79.
> > 1     F401 'stat' imported but unused
> > 1     W391 blank line at end of file
> > Signed-off-by: Nir Soffer
> >
> > README.dev |  4 ++++
> >  tox.ini    | 12 +++++++++++-
> >  2 files changed, 15 insertions(+), 1 deletion(-)
> >
> > diff --git a/README.dev b/README.dev
> > index cc2c613..8ade5e6 100644
> > --- a/README.dev
> > +++ b/README.dev
> > @@ -35,10 +35,14 @@ To run only test from some modules:
> >
> > To run only tests matching the substring "foo":
> >  $ tox -- -k foo
> >
> > +To run basic lint and style check:
> > $ tox -e flake8
> > Testing 4K support
> >
> > To enable the 4k tests, you need to setup 4k stroage for the tests:
>
> Typo. "storage"

Not related to this patch.
> > diff --git a/tox.ini b/tox.ini
> > index 5a48b23..ca63f55 100644
> > --- a/tox.ini
> > +++ b/tox.ini
> > @@ -2,11 +2,11 @@
> >  # in multiple virtualenvs. This configuration file will run the
> >  # test suite on all supported python versions. To use it, "pip install
> > tox"
> >  # and then run "tox" from this directory.
> >
> > [tox]
> > -envlist = py27,py36
> > +envlist = py27,py36,flake8
> >  skipsdist = True
> >  skip_missing_interpreters = True
> >
> > [testenv]
> >  passenv = *
> > @@ -20,12 +20,22 @@ deps =
> >  commands =
> >      py27: make PY_VERSION=2.7
> > BUILDARGS="--build-lib={envsitepackagesdir}"
> >      py36: make PY_VERSION=3.6
> > BUILDARGS="--build-lib={envsitepackagesdir}"
> >      pytest {posargs}
> >
> > +[testenv:flake8]
> > +deps = flake8
> > +commands = flake8 --statistics tests python
> >
> > [pytest]
> >  # Notes:
> >  # --basetemp: we must use /var/tmp as sanlock uses direct I/O.
> >  # -vv: increasing verbosify twice shows more detailed failures
> > tracebacks.
> >  # -rxs: show extra test summary: (s)skipped, (x)failed
> >  # --durations: show slowest test duration
> >  addopts = -rxs -vv --basetemp=/var/tmp/sanlock --durations=10
> >
> > +[flake8]
> > +# We like to use start imports units and constants.
>
> Typo. "star"

Thanks, will fix before pushing.
> > +# F403 'from .units import *' used; unable to detect undefined names
> > +# F405 'KiB' may be undefined, or defined from star imports: .units
>
> Just want to understand:
> F403 - That means that all "import *" except "from .units" will fail
>
> the build? If so, this is great, I don't like "import *"  :)
> 2) F405 - I understand that we explicitly ignore "KiB" error. Why? When
> the error happens? What about other constants?

The error was copied from flake8 --statistics output, so it is confusing.

F403 ignores errors about star imports, which is fine. F405 ignores errors
about undefined
variables, which is less useful.

I will remove these ignores in v3, since we really like to have warnings
about undefined variables.
> > +ignore = F403, F405
> >
> > 2.17.2