Closed Bug 1515746 Opened 6 years ago Closed 6 years ago

[flake8] Remove ability to specify subdir configs

Categories

(Developer Infrastructure :: Lint and Formatting, enhancement, P2)

Product:
Developer Infrastructure
Component:
Lint and Formatting
Type:
enhancement

Tracking

(firefox67 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla67
Tracking Flags:
Tracking Status
firefox67 --- fixed

People

(Reporter: ahal, Assigned: ahal)

References

Details

Attachments

(3 files)

Bug 1515746 - [lint] Remove duplicate flake8 implementation, r?egao
(deleted), text/x-phabricator-request
Details
Bug 1515746 - [flake8] Upgrade flake8 and dependencies, r?egao
(deleted), text/x-phabricator-request
Details
Bug 1515746 - [flake8] Unsupport subdir .flake8 files and use new 'per-file-ignores' config instead, r?egao
(deleted), text/x-phabricator-request
Details
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

https://hg.mozilla.org/mozilla-central/rev/ba9e0f7f49a3
https://hg.mozilla.org/mozilla-central/rev/3c7c50fba283
https://hg.mozilla.org/mozilla-central/rev/f96c1460ffc0

Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox67: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla67
Product: Firefox Build System → Developer Infrastructure
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: