Re: [PATCH v5 33/36] qapi/types.py: remove one-letter variables

[John Snow]

On 10/7/20 8:42 AM, Markus Armbruster wrote:
> John Snow <jsnow@redhat.com> writes:
>
> > "John, if pylint told you to jump off a bridge, would you?"
> > Hey, if it looked like fun, I might.
> >
> > Now that this file is clean, enable pylint checks on this file.
> >
> > Signed-off-by: John Snow <jsnow@redhat.com>
> > Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
> > Reviewed-by: Cleber Rosa <crosa@redhat.com>
> > ---
> >   scripts/qapi/pylintrc |  1 -
> >   scripts/qapi/types.py | 29 +++++++++++++++--------------
> >   2 files changed, 15 insertions(+), 15 deletions(-)
> >
> > diff --git a/scripts/qapi/pylintrc b/scripts/qapi/pylintrc
> > index 8badcb11cda..b3c4cf46dbf 100644
> > --- a/scripts/qapi/pylintrc
> > +++ b/scripts/qapi/pylintrc
> > @@ -6,7 +6,6 @@ ignore-patterns=error.py,
> >                   expr.py,
> >                   parser.py,
> >                   schema.py,
> > -                types.py,
> >                   visit.py,
> >   
> >   
> > diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py
> > index 766822feb3a..9d1e79d503d 100644
> > --- a/scripts/qapi/types.py
> > +++ b/scripts/qapi/types.py
> > @@ -49,14 +49,14 @@ def gen_enum_lookup(name: str,
> >       .array = (const char *const[]) {
> >   ''',
> >                   c_name=c_name(name))
> > -    for m in members:
> > -        ret += gen_if(m.ifcond)
> > -        index = c_enum_const(name, m.name, prefix)
> > +    for member in members:
>
> Let's use @memb.  It's more visually distinct from @members, and
> gen_struct_members() already uses it.

OK.

> > +        ret += gen_if(member.ifcond)
> > +        index = c_enum_const(name, member.name, prefix)
> >           ret += mcgen('''
> >           [%(index)s] = "%(name)s",
> >   ''',
> > -                     index=index, name=m.name)
> > -        ret += gen_endif(m.ifcond)
> > +                     index=index, name=member.name)
> > +        ret += gen_endif(member.ifcond)
> >   
> >       ret += mcgen('''
> >       },
> > @@ -79,13 +79,13 @@ def gen_enum(name: str,
> >   ''',
> >                   c_name=c_name(name))
> >   
> > -    for m in enum_members:
> > -        ret += gen_if(m.ifcond)
> > +    for member in enum_members:
> > +        ret += gen_if(member.ifcond)
> >           ret += mcgen('''
> >       %(c_enum)s,
> >   ''',
> > -                     c_enum=c_enum_const(name, m.name, prefix))
> > -        ret += gen_endif(m.ifcond)
> > +                     c_enum=c_enum_const(name, member.name, prefix))
> > +        ret += gen_endif(member.ifcond)
> >   
> >       ret += mcgen('''
> >   } %(c_name)s;
> > @@ -148,11 +148,12 @@ def gen_object(name: str, ifcond: List[str],
> >       objects_seen.add(name)
> >   
> >       ret = ''
> > -    if variants:
> > -        for v in variants.variants:
> > -            if isinstance(v.type, QAPISchemaObjectType):
> > -                ret += gen_object(v.type.name, v.type.ifcond, v.type.base,
> > -                                  v.type.local_members, v.type.variants)
> > +    for variant in variants.variants if variants else ():
>
> Let's use @var, for consistency with gen_variants().

You're the boss.

(Said only when I don't actually still kinda sorta want to do it my way. 
Then you're the boss, but in the bad way. Like I am the petulant 
teenager working at a Burger King and I am having a disagreement with 
the Assistant Manager over the proper way to stock the ketchup packets.)

> > +        obj = variant.type
> > +        if not isinstance(obj, QAPISchemaObjectType):
> > +            continue
> > +        ret += gen_object(obj.name, obj.ifcond, obj.base,
> > +                          obj.local_members, obj.variants)
> >   
> >       ret += mcgen('''