[flake8] Remove ability to specify subdir configs

RESOLVED FIXED in Firefox 67

Status

enhancement
P2
normal
RESOLVED FIXED
7 months ago
5 months ago

People

(Reporter: ahal, Assigned: ahal)

Tracking

Trunk
mozilla67
Dependency tree / graph

Firefox Tracking Flags

(firefox67 fixed)

Details

Attachments

(3 attachments)

There are a handful of directories in the tree that specify custom .flake8 files to override the default configuration:
https://searchfox.org/mozilla-central/search?q=.flake8&case=true&path=

Because .flake8 doesn't support this natively, this functionality is implemented in our own custom integration here:
https://searchfox.org/mozilla-central/rev/232ced2697b8938073fa79b8e6aa3718876c0b69/tools/lint/python/flake8.py#135

This is a problem because:

1) The logic is complex
2) It is blocking us from using a blacklist
3) It causes differences when running "flake8" without |mach lint| (annoying for people using editor integrations)

In flake8 3.7 (unreleased as of this writing), there will be the ability to specify custom configuration for subdirs directly in the root .flake8 file. This would let us get rid of all this sub-config logic and finally switch to a blacklist while at the same time making sure the native flake8 config is closer to the |mach| one.
Priority: -- → P2

Flake8 3.7.1 with the prerequisite feature was released yesterday.

Somehow we have two copies of the flake8 linter implementation:

- tools/lint/python/__init__.py
- tools/lint/python/flake8.py

I'm not really sure how this happened, but the latter is the one that we use,
so let's remove the former. It wasn't really affecting anything, though could
have caused confusion to people looking to modify the lint integration.

This bumps flake8 to version 3.7.3.

This also ignores the new lint rules that were added in the new versions.
These rules are de-marked via comment so we know that they should be enabled at
some point (as opposed to the other rules that are (presumably) ignored
intentionally.

Depends on D18352

This removes all .flake8 files except for the one at the root of the repo.
Instead we use the new 'per-file-ignores' config introduced in 3.7. To ignore
specific errors in a subdirectory, add a line like this to the root .flake8:

[per-file-ignores]
path/to/subdir/*: E100, F200, ...

The reasons for this change are:

  1. Unblock flake8 blacklist (bug 1367092).
  2. Simplify configuration and code.
  3. Encourage more consistent styling.
  4. Improve performance.
  5. Greater editor consistency.

Depends on D18353

Assignee: nobody → ahal
Status: NEW → ASSIGNED
Pushed by ahalberstadt@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ba9e0f7f49a3
[lint] Remove duplicate flake8 implementation, r=egao
https://hg.mozilla.org/integration/autoland/rev/3c7c50fba283
[flake8] Upgrade flake8 and dependencies, r=egao
https://hg.mozilla.org/integration/autoland/rev/f96c1460ffc0
[flake8] Unsupport subdir .flake8 files and use new 'per-file-ignores' config instead, r=egao
Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67
You need to log in before you can comment on or make changes to this bug.