[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 05/38] qapi: Remove wildcard includes
From: |
Cleber Rosa |
Subject: |
Re: [PATCH v2 05/38] qapi: Remove wildcard includes |
Date: |
Wed, 23 Sep 2020 09:27:35 -0400 |
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 <jsnow@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
> 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.
Reviewed-by: Cleber Rosa <crosa@redhat.com>
signature.asc
Description: PGP signature
- Re: [PATCH v2 07/38] qapi: add pylintrc, (continued)
[PATCH v2 09/38] qapi/common.py: Add indent manager, John Snow, 2020/09/22
[PATCH v2 05/38] qapi: Remove wildcard includes, John Snow, 2020/09/22
[PATCH v2 12/38] qapi/common.py: check with pylint, John Snow, 2020/09/22
[PATCH v2 04/38] qapi: Prefer explicit relative imports, John Snow, 2020/09/22