Closed Bug 1277851 Opened 5 years ago Closed 2 years ago
[mozlint] Better flake8 configuration support
58 bytes, text/x-review-board-request
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.
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)
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 email@example.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/91a825772c31 [mozlint] Run flake8 directories that contain a .flake8 file separately, r=maja_zf
I consider this more of a quick hack than long term solution, so marking [leave-open] to tackle this properly in the future.
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.
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.
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.