|
From: | John Snow |
Subject: | Re: [PATCH v2 05/38] qapi: Remove wildcard includes |
Date: | Wed, 23 Sep 2020 13:21:37 -0400 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.11.0 |
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 <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, ifcontextIs 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 QAPISchemaVisitordiff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.pyindex 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, QAPISchemaObjectTypediff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.pyindex 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.
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
Reviewed-by: Cleber Rosa <crosa@redhat.com>
Thanks! --js
[Prev in Thread] | Current Thread | [Next in Thread] |