[PATCH 2/2] u_boot_pylib: Clean up pylint warnings in gitutil.py

[Simon Glass]

Hi Tom,

On Sat, 15 Mar 2025 at 15:23, Tom Rini <trini at konsulko.com> wrote:
>
> On Sat, Mar 15, 2025 at 02:48:18PM +0000, Simon Glass wrote:
> > Hi Tom,
> >
> > On Fri, 14 Mar 2025 at 15:36, Tom Rini <trini at konsulko.com> wrote:
> > >
> > > On Fri, Mar 14, 2025 at 02:45:27PM +0000, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Mon, 17 Feb 2025 at 19:23, Tom Rini <trini at konsulko.com> wrote:
> > > > >
> > > > > On Sun, Feb 09, 2025 at 02:26:01PM -0700, Simon Glass wrote:
> > > > >
> > > > > > Reduce the number of warnings in this file.
> > > > > >
> > > > > > Signed-off-by: Simon Glass <sjg at chromium.org>
> > > > > > ---
> > > > > >
> > > > > >  tools/buildman/func_test.py   |   4 +-
> > > > > >  tools/u_boot_pylib/gitutil.py | 240 ++++++++++++++++++----------------
> > > > > >  2 files changed, 128 insertions(+), 116 deletions(-)
> > > > >
> > > > > As I think these all were warnings from whatever other linting you had
> > > > > mentioned in another thread, I deferred this patch as it also doesn't
> > > > > cleanly apply.
> > > >
> > > > Some of it, yes. But most of it is fixing things that we should fix.
> > >
> > > To be clear, to the extent any of it was pylint problems, they've now
> > > been addressed as we've moved to pylint 3.3.4.
> >
> > Actually, I don't know about those problems, but that's fine.
> >
> > >
> > > > This is going to open up a significant diff for patman, unfortunately,
> > > > just as I am adding new features for series management. So please
> > > > consider applying it. I can rebase if you like?
> > >
> > > Yes, that's fine, please just explain what's showing the warnings or
> > > otherwise update the commit message.
> >
> > Before:
> >
> > ************* Module u_boot_pylib.gitutil
> > tools/u_boot_pylib/gitutil.py:1:0: C0114: Missing module docstring
> > (missing-module-docstring)
> > tools/u_boot_pylib/gitutil.py:13:0: C0103: Constant name
> > "use_no_decorate" doesn't conform to UPPER_CASE naming style
> > (invalid-name)
> > tools/u_boot_pylib/gitutil.py:40:19: C0209: Formatting a regular
> > string which could be an f-string (consider-using-f-string)
> > tools/u_boot_pylib/gitutil.py:65:20: C0209: Formatting a regular
> > string which could be an f-string (consider-using-f-string)
> > tools/u_boot_pylib/gitutil.py:72:25: C0209: Formatting a regular
> > string which could be an f-string (consider-using-f-string)
> > tools/u_boot_pylib/gitutil.py:64:12: W0612: Unused variable 'msg'
> > (unused-variable)
> > tools/u_boot_pylib/gitutil.py:114:21: C0209: Formatting a regular
> > string which could be an f-string (consider-using-f-string)
> > tools/u_boot_pylib/gitutil.py:121:25: C0209: Formatting a regular
> > string which could be an f-string (consider-using-f-string)
> > tools/u_boot_pylib/gitutil.py:122:17: C0209: Formatting a regular
> > string which could be an f-string (consider-using-f-string)
> > tools/u_boot_pylib/gitutil.py:139:41: C0209: Formatting a regular
> > string which could be an f-string (consider-using-f-string)
> > tools/u_boot_pylib/gitutil.py:141:40: C0209: Formatting a regular
> > string which could be an f-string (consider-using-f-string)
> > tools/u_boot_pylib/gitutil.py:146:4: R1705: Unnecessary "elif" after
> > "return", remove the leading "el" from "elif" (no-else-return)
> > tools/u_boot_pylib/gitutil.py:151:15: C0209: Formatting a regular
> > string which could be an f-string (consider-using-f-string)
> > tools/u_boot_pylib/gitutil.py:153:25: C0209: Formatting a regular
> > string which could be an f-string (consider-using-f-string)
> > tools/u_boot_pylib/gitutil.py:171:11: C0209: Formatting a regular
> > string which could be an f-string (consider-using-f-string)
> > tools/u_boot_pylib/gitutil.py:189:21: C0209: Formatting a regular
> > string which could be an f-string (consider-using-f-string)
> > tools/u_boot_pylib/gitutil.py:243:22: C0209: Formatting a regular
> > string which could be an f-string (consider-using-f-string)
> > tools/u_boot_pylib/gitutil.py:255:22: C0209: Formatting a regular
> > string which could be an f-string (consider-using-f-string)
> > tools/u_boot_pylib/gitutil.py:272:22: C0209: Formatting a regular
> > string which could be an f-string (consider-using-f-string)
> > tools/u_boot_pylib/gitutil.py:305:22: C0209: Formatting a regular
> > string which could be an f-string (consider-using-f-string)
> > tools/u_boot_pylib/gitutil.py:317:22: C0209: Formatting a regular
> > string which could be an f-string (consider-using-f-string)
> > tools/u_boot_pylib/gitutil.py:320:0: R0913: Too many arguments (6/5)
> > (too-many-arguments)
> > tools/u_boot_pylib/gitutil.py:320:0: R0917: Too many positional
> > arguments (6/5) (too-many-positional-arguments)
> > tools/u_boot_pylib/gitutil.py:345:16: C0209: Formatting a regular
> > string which could be an f-string (consider-using-f-string)
> > tools/u_boot_pylib/gitutil.py:347:12: C0209: Formatting a regular
> > string which could be an f-string (consider-using-f-string)
> > tools/u_boot_pylib/gitutil.py:353:4: R1705: Unnecessary "else" after
> > "return", remove the "else" and de-indent the code inside it
> > (no-else-return)
> > tools/u_boot_pylib/gitutil.py:402:16: C0209: Formatting a regular
> > string which could be an f-string (consider-using-f-string)
> > tools/u_boot_pylib/gitutil.py:416:7: R1714: Consider merging these
> > comparisons with 'in' by using 'suppresscc in ('all', 'cccmd')'. Use a
> > set instead if elements are hashable. (consider-using-in)
> > tools/u_boot_pylib/gitutil.py:420:15: C0209: Formatting a regular
> > string which could be an f-string (consider-using-f-string)
> > tools/u_boot_pylib/gitutil.py:433:0: R0913: Too many arguments (12/5)
> > (too-many-arguments)
> > tools/u_boot_pylib/gitutil.py:433:0: R0917: Too many positional
> > arguments (12/5) (too-many-positional-arguments)
> > tools/u_boot_pylib/gitutil.py:433:0: R0914: Too many local variables
> > (17/15) (too-many-locals)
> > tools/u_boot_pylib/gitutil.py:512:19: C0209: Formatting a regular
> > string which could be an f-string (consider-using-f-string)
> > tools/u_boot_pylib/gitutil.py:514:19: C0209: Formatting a regular
> > string which could be an f-string (consider-using-f-string)
> > tools/u_boot_pylib/gitutil.py:520:24: C0209: Formatting a regular
> > string which could be an f-string (consider-using-f-string)
> > tools/u_boot_pylib/gitutil.py:433:0: R1710: Either all return
> > statements in a function should return an expression, or none of them
> > should. (inconsistent-return-statements)
> > tools/u_boot_pylib/gitutil.py:435:36: W0613: Unused argument
> > 'get_maintainer_script' (unused-argument)
> > tools/u_boot_pylib/gitutil.py:592:14: C0209: Formatting a regular
> > string which could be an f-string (consider-using-f-string)
> > tools/u_boot_pylib/gitutil.py:593:8: R1720: Unnecessary "else" after
> > "raise", remove the "else" and de-indent the code inside it
> > (no-else-raise)
> > tools/u_boot_pylib/gitutil.py:601:18: C0209: Formatting a regular
> > string which could be an f-string (consider-using-f-string)
> > tools/u_boot_pylib/gitutil.py:682:4: W0603: Using the global statement
> > (global-statement)
> >
> > ------------------------------------------------------------------
> > Your code has been rated at 8.41/10 (previous run: 8.37/10, +0.04)
> >
> >
> > After:
> >
> > ************* Module u_boot_pylib.gitutil
> > tools/u_boot_pylib/gitutil.py:1:0: C0114: Missing module docstring
> > (missing-module-docstring)
> > tools/u_boot_pylib/gitutil.py:325:0: R0913: Too many arguments (6/5)
> > (too-many-arguments)
> > tools/u_boot_pylib/gitutil.py:325:0: R0917: Too many positional
> > arguments (6/5) (too-many-positional-arguments)
> > tools/u_boot_pylib/gitutil.py:439:0: R0913: Too many arguments (11/5)
> > (too-many-arguments)
> > tools/u_boot_pylib/gitutil.py:439:0: R0917: Too many positional
> > arguments (11/5) (too-many-positional-arguments)
> > tools/u_boot_pylib/gitutil.py:439:0: R0914: Too many local variables
> > (16/15) (too-many-locals)
> > tools/u_boot_pylib/gitutil.py:694:4: W0603: Using the global statement
> > (global-statement)
> >
> > ------------------------------------------------------------------
> > Your code has been rated at 9.73/10 (previous run: 8.41/10, +1.32)
>
> And it's not clear where these warnings come from or why they matter or
> how to check things further.

Quite a few of them have been there for a while, but most of them are
coming from newer versions of pylint, where people come up with new
warnings. The f-string thing is the main one, since that feature was
not available when the code was written, but now you get a warning if
you don't use it, because Python.

>
> > The patch is here:
> >
> > https://patchwork.ozlabs.org/project/uboot/patch/20250209212714.2317040-2-sjg@chromium.org/
> >
> > It looks like it will apply for you, if first you accept my PR.
>
> It does, but I'm still not thrilled with the commit message.

I could do a new one with the info above if you like. We are going to
have this all over the place...

Regards,
Simon