[code-quality] Searching multiple setup.cfg files for flake8 section?

[Ian Cordasco]

On Wed, Jul 8, 2015 at 10:15 AM, Willy Wu <wwu at dropbox.com> wrote:
>
>
> On Wed, Jul 8, 2015 at 4:18 AM, Ian Cordasco <graffatcolmingov at gmail.com>
> wrote:
>>
>> On Tue, Jul 7, 2015 at 2:11 PM, Willy Wu <wwu at dropbox.com> wrote:
>> > So our flake8 might be a simpler case, where our lint tool invokes
>> > flake8 on
>> > every file in the diff individually rather than a bunch of files at
>> > once.
>> > Am I right to guess that if you do "flake8 .", then flake8 picks a
>> > single
>> > per-project config file to apply across all files?
>>
>> Like I said, we do not change how flake8 behaves whether its running
>> against a lot of files or whether it's running against a single file.
>>
>> When running flake8, it looks in the current working directory for a
>> config file to apply to all files that it then discovers.
>>
>> >> > I think the source of our confusion is that we use py.test as our
>> >> > unit
>> >> > test
>> >> > runner, and they only consider setup.cfg a match if it contains a
>> >> > [pytest]
>> >> > section, so there's kind of a tension between how you guys and pytest
>> >> > are
>> >> > resolving configs.
>> >>
>> >> Except it's much more in line with how tox resolves configs. We can
>> >> find points and counter-points but saying X should behave like Y is
>> >> not a discussion worth having unless Y is a canonical implementation
>> >> of something (and there really is no canon in this kind of
>> >> configuration discovery/selection for Python tooling that looks in
>> >> several places).
>> >>
>> >> > Let me know what else you need?
>> >>
>> >> Which setup.cfg has the Flake8 config? Is it
>> >>
>> >> - {repo_root}/setup.cfg
>> >> - {repo_root}/{subservice_x}/setup.cfg
>> >> - Something else?
>> >
>> >
>> > {repo_root}/setup.cfg is originally the file w/ flake8 configs, and it's
>> > where we were hoping to consolidate toward.
>>
>> And you're saying that doesn't work right now? I'm confused. That
>> should Just Work™ already. What won't work is shoving the config into
>> the setup.cfg files in the subservices.
>
>
> I made a gist with working shell code, it'll be easier for everyone if I
> gave an example.  :)  It looks like the behavior for "flake8 ." vs "flake8
> {path}" is different, which is probably why we're both kinda confused.
>
> https://gist.github.com/willywu/f18211d1977a7189402b
>
> Notice that if the root's setup.cfg is the only file w/ flake8 configs, then
> `flake8 subproject1/src/myfile.py` doesn't use the root configs and gives an
> error.  But when I put the flake8 configs inside of subproject1/setup.cfg,
> then we get the behavior that I was hoping for.
>
> But with that said, I think a totally viable workaround is to run `flake8
> subproject1/src/myfile.py --config=setup.cfg` which also does the behavior
> that I'm hoping for.
>
> Soo...seems to be that if you invoke flake8 with a path to a file, flake8
> will stop at the first discovered setup.cfg (my original issue).  But that's
> atleast how pep8/tox works, even if it's not how py.test works.  You guys
> should update the docs on
> http://flake8.readthedocs.org/en/latest/config.html to mention the --config
> param!  Because why would someone run `flake8 --help` instead of reading the
> web docs...

This is awesome. I was about to start trying permutations of this to
really grok what you were encountering/try to reproduce it. This gives
me a clear set of steps.

So flake8 takes full advantage of pep8's config handling. (Which you
can skim here: https://github.com/jcrocholl/pep8/blob/a6e423cd4e972b19dc1d2c3d6db6269c2c445151/pep8.py#L1985)

If I had read things closer I would have seen
https://github.com/jcrocholl/pep8/blob/a6e423cd4e972b19dc1d2c3d6db6269c2c445151/pep8.py#L1998
but I was confused and not reading the code clearly. Sorry about that,
this confusion is probably my fault.

At this point, I'm not sure we can fix that, although I agree that it
isn't ideal. We should probably attempt to read the config file into a
separate config parser, check if it has a relevant section and /then/
read it into our actual config object and break. That said, the config
parsing part of pep8 seems to be very fragile (as is evidenced by pep8
1.6.0, 1.6.1, and 1.6.2). If Ian (Lee) is up for it, I'd love to work
with him to carefully fix this and other config discovery/parsing bugs
that we know about in pep8 for a pep8 2.0 release. I'd also like to
see at least one more 1.x release if not one more 1.6.x release that
flake8 users can consume with the reverted changes from the current
1.6.x branch.

I'll file a bug about `--config` not being covered in our web docs,
but I think I might just remove that stuff altogether. It's too easy
for it to get out of date and extensions can add extra flags that will
effectively make that output wrong for users who do not check "flake8
--help".

--
Ian