[prev in list] [next in list] [prev in thread] [next in thread] 

List:       gentoo-portage-dev
Subject:    Re: [gentoo-portage-dev] pylint progress
From:       Zac Medico <zmedico () gentoo ! org>
Date:       2020-07-30 7:44:39
Message-ID: 8643487f-c90f-ec37-0674-5088648c1b6d () gentoo ! org
[Download RAW message or body]

[Attachment #2 (multipart/mixed)]


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)]

[prev in list] [next in list] [prev in thread] [next in thread]