Closed
Bug 1277851
Opened 8 years ago
Closed 6 years ago
[mozlint] Better flake8 configuration support
Categories
(Developer Infrastructure :: Lint and Formatting, defect, P5)
Developer Infrastructure
Lint and Formatting
Tracking
(Not tracked)
RESOLVED
WONTFIX
People
(Reporter: ahal, Assigned: ahal)
References
Details
Attachments
(1 file)
It turns out that just adding new .flake8 files to subdirectories doesn't work out as nicely as I said it did. I should have caught this earlier. Here's some extra context.
Flake8 currently defers all its configuration handling to the pep8 module, and pep8 handles configuration really stupidly. It will use the first configuration file that is common to *all specified paths*.
So if you run:
flake8 testing/marionette/client
Then it will use 'testing/marionette/client/.flake8'. But if you specify a second path, e.g:
flake8 testing/marionette/client tools/lint
Then it will search for a single common config file (which in our case is the global .flake8 file). This is incredibly dumb, I should have noticed this earlier. There is some bad news and good news.
The bad news is that pep8 as a project is stalled out, and the likelihood of them either implementing multiple configs or accepting multiple configs is pretty low. The good news is that flake8 is planning a version 3 which aims to stop using pep8 for things like configuration, and multiple config files is something they're at least interested in.
So until flake8 3 comes out with better config file support, we have two options:
1) Invoke a new flake8 subprocess for each .flake8 file we find, this will involve walking the tree of each file path we pass in.
2) Only use the global config (no subdirectory .flake8 files)
I'll take a quick look into 1) to see how feasible it is.
Assignee | ||
Comment 1•8 years ago
|
||
This is a crude workaround to get subdirectory .flake8 files working. Hopefully
this is temporary until flake8 3.0 is released with support for multiple config
files.
Note that .flake8 files that live outside of a directory that is explicitly
listed in the 'include' directive, will not be considered.
Review commit: https://reviewboard.mozilla.org/r/57590/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/57590/
Attachment #8759686 -
Flags: review?(dburns)
Assignee | ||
Updated•8 years ago
|
Attachment #8759686 -
Flags: review?(dburns) → review?(mjzffr)
Comment on attachment 8759686 [details]
Bug 1277851 - [mozlint] Run flake8 directories that contain a .flake8 file separately,
https://reviewboard.mozilla.org/r/57590/#review54392
Attachment #8759686 -
Flags: review?(mjzffr) → review+
Pushed by ahalberstadt@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/91a825772c31
[mozlint] Run flake8 directories that contain a .flake8 file separately, r=maja_zf
Assignee | ||
Comment 4•8 years ago
|
||
I consider this more of a quick hack than long term solution, so marking [leave-open] to tackle this properly in the future.
Whiteboard: [leave-open]
Comment 5•8 years ago
|
||
bugherder |
Assignee | ||
Updated•8 years ago
|
Component: General → Lint
Assignee | ||
Comment 6•7 years ago
|
||
Flake8 no longer relies on pep8's configuration, but this is still a problem. For posterity, I filed an issue about this a year ago:
https://gitlab.com/pycqa/flake8/issues/143
It looks unlikely that feature request will be accepted though. Maybe a plugin could implement something like that.
Updated•7 years ago
|
Priority: -- → P5
Assignee | ||
Comment 7•7 years ago
|
||
The yamllint linter has a similar problem to this, and a fix just landed to make it a bit better in bug 1392795. What it does is preprocess the paths by searching the ancestors of each path passed in for .yamllint configs. It then groups all paths that share the same configuration file and runs them together.
We should do this for flake8 as well. It still isn't ideal, but would be a big improvement on the status quo.
Updated•7 years ago
|
Product: Testing → Firefox Build System
Updated•6 years ago
|
Keywords: leave-open
Whiteboard: [leave-open]
Assignee | ||
Comment 8•6 years ago
|
||
Instead of this, I've decided to go the easier route of removing subdirectory .flake8 files. This is now possible because flake8 3.7 introduced a new feature that allows us to ignore specific errors in subdirectories in the root config file.
WONTFIXing in favour of bug 1515746.
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → WONTFIX
Assignee | ||
Updated•6 years ago
|
Keywords: leave-open
Updated•2 years ago
|
Product: Firefox Build System → Developer Infrastructure
You need to log in
before you can comment on or make changes to this bug.
Description
•