Re: pylint progress

Zac Medico <[email protected]> Thu, 30 Jul 2020 00:44:39 -0700
Newsgroups	gmane.linux.gentoo.portage.devel
Message-ID	<[email protected]>
On 7/29/20 3:14 PM, Alec Warner wrote:
> Hi,
> 
> Recently I've begun to run pylint on the portage codebase. You can see
> some recent PRs on this[0][1][2]. Most of the linter errors I've
> fixed are what I consider 'fairly trivial'. In general I'm happy to
> disable errors (or instances of errors) in addition to resolving them.
> You can see some of this
> in https://github.com/gentoo/portage/pull/593/files where we just
> blanket disable a bunch of very numerous messages (either because I
> don't expect to ever fix them, or because they are more work that I
> think we should do at the moment.)
> 
> So I see essentially a few choices:
>  - (a) Do we fix errors in certain classes and run the linter as part of
> CI. This means that once we resolve errors of a certain class, we can
> enable that class of check and prevent new occurrences.

Yes, this is an extremely useful feature to included in our CI.

>  - (b) Do we just avoid running the linter as part of CI (perhaps
> because we are not far enough along on this journey) and focus our
> efforts to get to a point where we can do (a) without spamming CI?

I think it will be best if we arrange it so that we're not spammed with
recurring messages in every CI run. Otherwise, useful messages will be
difficult to recognize since they'll be drowned in noise.

>  - (c) What messages should we bother not fixing at all so we can just
> not work on those classes of error?
> 
> As an example of (c); I believe 'protected-access' is likely intrusive
> to fix and pointless for portage (cat is out of the bag) where as a
> message like W0612(unused-variable) is plausibly something we should fix
> throughout the codebase.

Sounds good.

> Similarly I think "E0602(undefined-variable)"
> is often a bug in how pylint treats our lazy initialized variables, so
> most of these are false positives.

I think it would be nice if we could enable the undefined-variable check.
-- 
Thanks,
Zac
signature.asc (application/pgp-signature, 981 B)