Re: [PATCH] scripts/qapi: minor delinting

From: Markus Armbruster <armbru@redhat.com>
To: John Snow <jsnow@redhat.com>
Cc: Michael Roth <michael.roth@amd.com>, qemu-devel@nongnu.org
Subject: Re: [PATCH] scripts/qapi: minor delinting
Date: Thu, 10 Feb 2022 16:56:20 +0100	[thread overview]
Message-ID: <87leyibtqz.fsf@pond.sub.org> (raw)
In-Reply-To: <20220204225230.2114135-1-jsnow@redhat.com> (John Snow's message of "Fri, 4 Feb 2022 17:52:30 -0500")

John Snow <jsnow@redhat.com> writes:

> Just cleaning up some cobwebs.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  scripts/qapi/commands.py | 2 +-
>  scripts/qapi/events.py   | 6 +++---
>  scripts/qapi/types.py    | 6 +++++-
>  scripts/qapi/visit.py    | 6 +++++-
>  4 files changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
> index 869d799ed2..38ca38a7b9 100644
> --- a/scripts/qapi/commands.py
> +++ b/scripts/qapi/commands.py
> @@ -25,8 +25,8 @@
>      QAPIGenC,
>      QAPISchemaModularCVisitor,
>      build_params,
> -    ifcontext,
>      gen_special_features,
> +    ifcontext,
>  )
>  from .schema import (
>      QAPISchema,
> diff --git a/scripts/qapi/events.py b/scripts/qapi/events.py
> index 27b44c49f5..8edf43d8da 100644
> --- a/scripts/qapi/events.py
> +++ b/scripts/qapi/events.py
> @@ -109,15 +109,15 @@ def gen_event_send(name: str,
>          if not boxed:
>              ret += gen_param_var(arg_type)
>  
> -    for f in features:
> -        if f.is_special():
> +    for feat in features:
> +        if feat.is_special():
>              ret += mcgen('''
>  
>      if (compat_policy.%(feat)s_output == COMPAT_POLICY_OUTPUT_HIDE) {
>          return;
>      }
>  ''',
> -                         feat=f.name)
> +                         feat=feat.name)
>  
>      ret += mcgen('''
>  

Meh.  Expressive variable names are good when there's something useful
to express.  But what's the added value in such a simple, obvious loop?

Besides:

    $ git-grep 'for . in' scripts/qapi | wc -l
    42
    $ git-grep -E 'for [A-Za-z0-9]{2,} in' scripts/qapi | wc -l
    31

> diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py
> index 3013329c24..477d027001 100644
> --- a/scripts/qapi/types.py
> +++ b/scripts/qapi/types.py
> @@ -16,7 +16,11 @@
>  from typing import List, Optional
>  
>  from .common import c_enum_const, c_name, mcgen
> -from .gen import QAPISchemaModularCVisitor, gen_special_features, ifcontext
> +from .gen import (
> +    QAPISchemaModularCVisitor,
> +    gen_special_features,
> +    ifcontext,
> +)
>  from .schema import (
>      QAPISchema,
>      QAPISchemaEnumMember,
> diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
> index e13bbe4292..380fa197f5 100644
> --- a/scripts/qapi/visit.py
> +++ b/scripts/qapi/visit.py
> @@ -21,7 +21,11 @@
>      indent,
>      mcgen,
>  )
> -from .gen import QAPISchemaModularCVisitor, gen_special_features, ifcontext
> +from .gen import (
> +    QAPISchemaModularCVisitor,
> +    gen_special_features,
> +    ifcontext,
> +)
>  from .schema import (
>      QAPISchema,
>      QAPISchemaEnumMember,

Everything else, gladly
Reviewed-by: Markus Armbruster <armbru@redhat.com>