1492495 - Enable 'flake8-isort' plugin in our flake8 linter

Status: RESOLVED FIXED

(Developer Infrastructure :: Lint and Formatting, enhancement)

Description

[Andrew Halberstadt [:ahal]]

It's a good idea to standardize on an import scheme in python. This will make it easy to see which libraries and modules are used in a file at a glance. The format I typically try to follow is:

<stdlib imports>

<library imports>

<package imports>

There is a flake8 plugin (flake8-import-order) that enforces this (the default style `cryptography` looks like it's already very close to what we use.

I think these issues should be logged as "warnings" to start. Making these "warnings" instead of "errors" means that they won't cause any backouts, and will only show up if you run:
./mach lint --warnings

This is helpful to us because we'll be able to get this landed without fixing all of the previously existing errors. Also since ordering your imports correctly is pretty nit-picky, we don't want to be too annoying with what causes a backout. Once this is landed, we can always re-evaluate if some or all issues should be converted to errors in the future.

The steps to fixing this bug are:

1) Add `flake8-import-order` to tools/lint/python/flake8_requirements.txt (hint: use the `hashin` module to generate hashes)
2) Verify it's working with ./mach lint -l flake8 (you should see lots of errors)
3) Modify tools/lint/python/flake8.py so issues generated by `flake8-import-order` are stored as warnings instead of the default 'error'
4) Add a test to tools/lint/test/test_flake8.py which makes sure an import order violation is picked up and that it's only a warning

Comment 1

[Andrew Halberstadt [:ahal]]

To test your changes:

1) Run ./mach lint -l flake8 (you shouldn't see any errors, but the summary will note lots of warnings)
2) Run ./mach lint -l flake8 --warnings (you should see all the warnings)
3) Run ./mach python-test flake8

Comment 2

[Andrew Halberstadt [:ahal]]

I think it would be better to use flake8-isort on second thought. Isort is an in-place formatter and flake8-isort is a wrapper around that which uses it more as a linter in flake8.

This means we would get fixing for free, which will be a huge help enabling this as a non-warning across the tree.

Comment 3

[Marco Castelluccio [:marco]]

Attachment: Bug 1492495 - Add flake8-isort plugin to sort Python includes, with support for autofixing through isort. r=#linter-reviewers

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e9f0f29e3ad0
Add flake8-isort plugin to sort Python includes, with support for autofixing through isort. r=linter-reviewers,ahal

Comment 5

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/e9f0f29e3ad0