On Wed, Sep 23, 2020 at 01:21:37PM -0400, John Snow wrote: > On 9/23/20 9:27 AM, Cleber Rosa wrote: > > On Tue, Sep 22, 2020 at 05:00:28PM -0400, John Snow wrote: > > > Wildcard includes become hard to manage when refactoring and dealing > > > with circular dependencies with strictly typed mypy. > > > > > > flake8 also flags each one as a warning, as it is not smart enough to > > > know which names exist in the imported file. > > > > > > Remove them and include things explicitly by name instead. > > > > > > Signed-off-by: John Snow <js...@redhat.com> > > > --- > > > scripts/qapi/commands.py | 6 +++++- > > > scripts/qapi/events.py | 7 ++++++- > > > scripts/qapi/gen.py | 12 +++++++++--- > > > scripts/qapi/introspect.py | 7 ++++++- > > > scripts/qapi/types.py | 8 +++++++- > > > scripts/qapi/visit.py | 10 +++++++++- > > > 6 files changed, 42 insertions(+), 8 deletions(-) > > > > > > diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py > > > index ce5926146a..e1df0e341f 100644 > > > --- a/scripts/qapi/commands.py > > > +++ b/scripts/qapi/commands.py > > > @@ -13,7 +13,11 @@ > > > See the COPYING file in the top-level directory. > > > """ > > > -from .common import * > > > +from .common import ( > > > + build_params, > > > + c_name, > > > + mcgen, > > > +) > > > from .gen import QAPIGenCCode, QAPISchemaModularCVisitor, ifcontext > > > > Is this import style being suggested or enforced by any tool? I've > > been using isort with very good results (both as a check tool, and as > > an emacs extension). For instance, the block about would look like: > > > > from .common import build_params, c_name, mcgen > > from .gen import QAPIGenCCode, QAPISchemaModularCVisitor, ifcontext > > > > Not enforced by any tool, no. Just subjective preference for git-friendly > import lines. They conflict on rebase a lot less. > > I have been using emacs sort-lines to order the names in a group. > > > > diff --git a/scripts/qapi/events.py b/scripts/qapi/events.py > > > index 0467272438..6b3afa14d7 100644 > > > --- a/scripts/qapi/events.py > > > +++ b/scripts/qapi/events.py > > > @@ -12,7 +12,12 @@ > > > See the COPYING file in the top-level directory. > > > """ > > > -from .common import * > > > +from .common import ( > > > + build_params, > > > + c_enum_const, > > > + c_name, > > > + mcgen, > > > +) > > > from .gen import QAPISchemaModularCVisitor, ifcontext > > > from .schema import QAPISchemaEnumMember > > > from .types import gen_enum, gen_enum_lookup > > > diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py > > > index 8df19a0df0..11472ba043 100644 > > > --- a/scripts/qapi/gen.py > > > +++ b/scripts/qapi/gen.py > > > @@ -11,13 +11,19 @@ > > > # This work is licensed under the terms of the GNU GPL, version 2. > > > # See the COPYING file in the top-level directory. > > > - > > > +from contextlib import contextmanager > > > import errno > > > import os > > > import re > > > -from contextlib import contextmanager > > > -from .common import * > > > +from .common import ( > > > + c_fname, > > > + gen_endif, > > > + gen_if, > > > + guardend, > > > + guardstart, > > > + mcgen, > > > +) > > > from .schema import QAPISchemaVisitor > > > diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py > > > index 2a34cd1e8e..b036fcf9ce 100644 > > > --- a/scripts/qapi/introspect.py > > > +++ b/scripts/qapi/introspect.py > > > @@ -10,7 +10,12 @@ > > > See the COPYING file in the top-level directory. > > > """ > > > -from .common import * > > > +from .common import ( > > > + c_name, > > > + gen_endif, > > > + gen_if, > > > + mcgen, > > > +) > > > from .gen import QAPISchemaMonolithicCVisitor > > > from .schema import (QAPISchemaArrayType, QAPISchemaBuiltinType, > > > QAPISchemaType) > > > diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py > > > index ca9a5aacb3..53b47f9e58 100644 > > > --- a/scripts/qapi/types.py > > > +++ b/scripts/qapi/types.py > > > @@ -13,7 +13,13 @@ > > > # See the COPYING file in the top-level directory. > > > """ > > > -from .common import * > > > +from .common import ( > > > + c_enum_const, > > > + c_name, > > > + gen_endif, > > > + gen_if, > > > + mcgen, > > > +) > > > from .gen import QAPISchemaModularCVisitor, ifcontext > > > from .schema import QAPISchemaEnumMember, QAPISchemaObjectType > > > diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py > > > index 7850f6e848..ea277e7704 100644 > > > --- a/scripts/qapi/visit.py > > > +++ b/scripts/qapi/visit.py > > > @@ -13,7 +13,15 @@ > > > See the COPYING file in the top-level directory. > > > """ > > > -from .common import * > > > +from .common import ( > > > + c_enum_const, > > > + c_name, > > > + gen_endif, > > > + gen_if, > > > + mcgen, > > > + pop_indent, > > > + push_indent, > > > +) > > > > And here, isort will add the paranthesis (it does so based on space > > demands): > > > > from .common import (c_enum_const, c_name, gen_endif, gen_if, mcgen, > > pop_indent, push_indent) > > from .gen import QAPISchemaModularCVisitor, ifcontext > > from .schema import QAPISchemaObjectType > > > > Other than those suggestions, it LGTM. > > > > OK. We can add a check that asserts that isort(file) == file to keep our > include regimes consistent. I'll look into the tool, but it will be after > this marathon of a series. >
That goes without saying! > Here's a gitlab issue I made on my QEMU fork to help me keep track of > Python-related issues that I intend to use: > https://gitlab.com/jsnow/qemu/-/issues/6 > Nice! - Cleber. > > Reviewed-by: Cleber Rosa <cr...@redhat.com> > > > > Thanks! > > --js
signature.asc
Description: PGP signature
