RE: [PATCH v4 00/22] git-p4: Various code tidy-ups

From: Joel Holdsworth <jholdsworth@nvidia.com>
Date: Apr 4, 2022, 14:46 UTC
Message-ID: <BL0PR12MB484938E9950EBCA08315544CC8E59@BL0PR12MB4849.namprd12.prod.outlook.com>
In-Reply-To: <CAPMMpohZxpMFc-rVE96QbeGzN6NdF5CdYVp6FLrHD6Ngi=mu4A@mail.gmail.com>

Show 6 quoted lines

> I've tried to run pylint on the previous state and state after this patch, but
> unfortunately there's a *lot* of noise either way - from all the "pylint:
> disable" entries in the script I have to assume that at some point it was
> compliant with *some* pylint version, but at the moment it's very far from
> any sort of compliance with checks I can run (both before and after this
> patchset).

I'm using pylint 2.12.2 on Python 3. I agree there are still many pylint warnings - especially if we start trying to cut down the disable list. However, quite a few of them are false positives.

The majority of the unsuppressed warnings I get from pylint suggest changes requiring Python 3 e.g. conversion to F-Strings.

> 1) Is there a tool that can be used to check for PEP8 compliance, and shows
> only the two remaining issues you highlighted above?

I used autopep8 to locate the issue I fixed here. It's not nearly as thorough as pylint, but PEP8 itself is very patchy in regard to the issues the authors chose to address.

> 3) Which of the "pylint: disable" entries remain meaningful after these
> changes, if any? I imagine "disable=wrong-import-order" for example makes
> no sense?

I don't know if it is possible to cut the list down any more yet. However, "wrong-import-order" certainly is required to suppress warnings about the Python version check mixed into the import list.

> I personally have an interest in making sure this script keeps running correctly
> under python2, so I plan to test this when I can. I imagine this is already
> accounted for in the t98xx suite somewhere, but I haven't found it.

Can you give me a bit more information about this, because I was going to try and push for the git-p4 to discard Python 2 support later this year in a bid to simplify the code, reduce the testing burden, and move toward cleaner pylint output.

Thanks for your comments Joel