From: Cleber Rosa <crosa@redhat.com>
To: John Snow <jsnow@redhat.com>
Cc: "Peter Maydell" <peter.maydell@linaro.org>,
"Eduardo Habkost" <ehabkost@redhat.com>,
qemu-devel@nongnu.org, "Michael Roth" <mdroth@linux.vnet.ibm.com>,
"Markus Armbruster" <armbru@redhat.com>,
"Alex Bennée" <alex.bennee@linaro.org>
Subject: Re: [PATCH v2 05/38] qapi: Remove wildcard includes
Date: Thu, 24 Sep 2020 15:27:32 -0400 [thread overview]
Message-ID: <20200924192732.GC347036@localhost.localdomain> (raw)
In-Reply-To: <fcf633f4-c0f9-984b-ba84-acc14851ee72@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 5996 bytes --]
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 <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
> >
>
> 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 <crosa@redhat.com>
> >
>
> Thanks!
>
> --js
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
next prev parent reply other threads:[~2020-09-24 19:29 UTC|newest]
Thread overview: 190+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-22 21:00 [PATCH v2 00/38] qapi: static typing conversion, pt1 John Snow
2020-09-22 21:00 ` [PATCH v2 01/38] [DO-NOT-MERGE] qapi: add debugging tools John Snow
2020-09-22 23:43 ` Cleber Rosa
2020-09-23 16:48 ` John Snow
2020-09-22 21:00 ` [PATCH v2 02/38] qapi-gen: Separate arg-parsing from generation John Snow
2020-09-22 21:19 ` Eduardo Habkost
2020-09-23 0:00 ` Cleber Rosa
2020-09-23 17:05 ` John Snow
2020-09-24 19:24 ` Cleber Rosa
2020-09-25 11:34 ` Markus Armbruster
2020-09-25 15:37 ` John Snow
2020-09-28 11:45 ` Markus Armbruster
2020-09-22 21:00 ` [PATCH v2 03/38] qapi: move generator entrypoint into module John Snow
2020-09-22 21:23 ` Eduardo Habkost
2020-09-23 17:09 ` John Snow
2020-09-23 0:29 ` Cleber Rosa
2020-09-22 21:00 ` [PATCH v2 04/38] qapi: Prefer explicit relative imports John Snow
2020-09-22 21:25 ` Eduardo Habkost
2020-09-23 13:18 ` Cleber Rosa
2020-09-23 17:12 ` John Snow
2020-09-24 19:25 ` Cleber Rosa
2020-09-24 22:17 ` Beraldo Leal
2020-09-24 22:36 ` Cleber Rosa
2020-09-22 21:00 ` [PATCH v2 05/38] qapi: Remove wildcard includes John Snow
2020-09-22 21:37 ` Eduardo Habkost
2020-09-23 13:27 ` Cleber Rosa
2020-09-23 17:21 ` John Snow
2020-09-24 19:27 ` Cleber Rosa [this message]
2020-09-24 20:04 ` John Snow
2020-09-22 21:00 ` [PATCH v2 06/38] qapi: delint using flake8 John Snow
2020-09-22 21:43 ` Eduardo Habkost
2020-09-23 13:37 ` Cleber Rosa
2020-09-22 21:00 ` [PATCH v2 07/38] qapi: add pylintrc John Snow
2020-09-22 21:54 ` Eduardo Habkost
2020-09-23 13:42 ` Cleber Rosa
2020-09-23 17:23 ` John Snow
2020-09-24 19:29 ` Cleber Rosa
2020-09-22 21:00 ` [PATCH v2 08/38] qapi/common.py: Remove python compatibility workaround John Snow
2020-09-22 22:05 ` Eduardo Habkost
2020-09-23 14:37 ` Cleber Rosa
2020-09-22 21:00 ` [PATCH v2 09/38] qapi/common.py: Add indent manager John Snow
2020-09-22 22:22 ` Eduardo Habkost
2020-09-23 17:29 ` John Snow
2020-09-25 11:51 ` Markus Armbruster
2020-09-25 13:13 ` Eduardo Habkost
2020-09-25 13:47 ` Markus Armbruster
2020-09-25 14:42 ` John Snow
2020-09-23 14:55 ` Cleber Rosa
2020-09-23 17:30 ` John Snow
2020-09-25 11:55 ` Markus Armbruster
2020-09-25 15:41 ` John Snow
2020-09-22 21:00 ` [PATCH v2 10/38] qapi/common.py: delint with pylint John Snow
2020-09-22 21:46 ` Eduardo Habkost
2020-09-23 16:01 ` Cleber Rosa
2020-09-23 17:37 ` John Snow
2020-09-24 19:30 ` Cleber Rosa
2020-09-22 21:00 ` [PATCH v2 11/38] qapi/common.py: Replace one-letter 'c' variable John Snow
2020-09-22 22:24 ` Eduardo Habkost
2020-09-23 16:15 ` Cleber Rosa
2020-09-22 21:00 ` [PATCH v2 12/38] qapi/common.py: check with pylint John Snow
2020-09-22 22:26 ` Eduardo Habkost
2020-09-23 16:18 ` Cleber Rosa
2020-09-23 16:30 ` John Snow
2020-09-22 21:00 ` [PATCH v2 13/38] qapi/common.py: add type hint annotations John Snow
2020-09-22 22:44 ` Eduardo Habkost
2020-09-23 17:57 ` John Snow
2020-09-23 18:19 ` Eduardo Habkost
2020-09-23 19:28 ` Cleber Rosa
2020-09-22 21:00 ` [PATCH v2 14/38] qapi/common.py: Convert comments into docstrings, and elaborate John Snow
2020-09-23 14:22 ` Eduardo Habkost
2020-09-23 19:38 ` Cleber Rosa
2020-09-23 21:18 ` John Snow
2020-09-25 17:02 ` Cleber Rosa
2020-09-25 17:13 ` John Snow
2020-09-22 21:00 ` [PATCH v2 15/38] qapi/common.py: move build_params into gen.py John Snow
2020-09-23 14:26 ` Eduardo Habkost
2020-09-23 20:04 ` Cleber Rosa
2020-09-22 21:00 ` [PATCH v2 16/38] qapi: establish mypy type-checking baseline John Snow
2020-09-23 14:29 ` Eduardo Habkost
2020-09-23 20:11 ` Cleber Rosa
2020-09-22 21:00 ` [PATCH v2 17/38] qapi/events.py: add type hint annotations John Snow
2020-09-23 14:31 ` Eduardo Habkost
2020-09-23 20:18 ` Cleber Rosa
2020-09-22 21:00 ` [PATCH v2 18/38] qapi/events.py: Move comments into docstrings John Snow
2020-09-23 14:48 ` Eduardo Habkost
2020-09-23 18:21 ` John Snow
2020-09-25 12:19 ` Markus Armbruster
2020-09-25 15:55 ` John Snow
2020-09-28 11:49 ` Markus Armbruster
2020-09-28 15:04 ` John Snow
2020-09-23 20:19 ` Cleber Rosa
2020-09-22 21:00 ` [PATCH v2 19/38] qapi/commands.py: Don't re-bind to variable of different type John Snow
2020-09-23 14:48 ` Eduardo Habkost
2020-09-23 20:21 ` Cleber Rosa
2020-09-22 21:00 ` [PATCH v2 20/38] qapi/commands.py: add notational type hints John Snow
2020-09-23 14:50 ` Eduardo Habkost
2020-09-23 18:23 ` John Snow
2020-09-23 22:17 ` Cleber Rosa
2020-09-22 21:00 ` [PATCH v2 21/38] qapi/commands.py: enable checking with mypy John Snow
2020-09-23 14:51 ` Eduardo Habkost
2020-09-23 22:21 ` Cleber Rosa
2020-09-23 23:50 ` John Snow
2020-09-22 21:00 ` [PATCH v2 22/38] qapi/source.py: add type hint annotations John Snow
2020-09-23 14:52 ` Eduardo Habkost
2020-09-23 22:36 ` Cleber Rosa
2020-09-23 23:55 ` John Snow
2020-09-25 12:22 ` Markus Armbruster
2020-09-25 16:20 ` John Snow
2020-09-25 17:05 ` Cleber Rosa
2020-09-25 17:20 ` John Snow
2020-09-22 21:00 ` [PATCH v2 23/38] qapi/source.py: delint with pylint John Snow
2020-09-23 14:59 ` Eduardo Habkost
2020-09-23 22:41 ` Cleber Rosa
2020-09-22 21:00 ` [PATCH v2 24/38] qapi/gen.py: Fix edge-case of _is_user_module John Snow
2020-09-23 15:17 ` Eduardo Habkost
2020-09-23 18:29 ` John Snow
2020-09-23 18:33 ` Eduardo Habkost
2020-09-23 23:10 ` Cleber Rosa
2020-09-23 23:13 ` Cleber Rosa
2020-09-25 13:00 ` Markus Armbruster
2020-09-25 15:15 ` Eduardo Habkost
2020-09-25 15:50 ` Eduardo Habkost
2020-09-28 12:04 ` Markus Armbruster
2020-09-25 16:29 ` John Snow
2020-09-23 23:08 ` Cleber Rosa
2020-09-23 23:13 ` Cleber Rosa
2020-09-23 23:57 ` John Snow
2020-09-22 21:00 ` [PATCH v2 25/38] qapi/gen.py: add type hint annotations John Snow
2020-09-23 15:18 ` Eduardo Habkost
2020-09-23 23:51 ` Cleber Rosa
2020-09-24 0:29 ` John Snow
2020-09-24 1:29 ` Eduardo Habkost
2020-09-22 21:00 ` [PATCH v2 26/38] qapi/gen.py: Enable checking with mypy John Snow
2020-09-23 15:18 ` Eduardo Habkost
2020-09-23 23:59 ` Cleber Rosa
2020-09-22 21:00 ` [PATCH v2 27/38] qapi/gen.py: Remove unused parameter John Snow
2020-09-23 15:19 ` Eduardo Habkost
2020-09-24 0:00 ` Cleber Rosa
2020-09-22 21:00 ` [PATCH v2 28/38] qapi/gen.py: update write() to be more idiomatic John Snow
2020-09-23 15:26 ` Eduardo Habkost
2020-09-23 18:37 ` John Snow
2020-09-24 15:59 ` Cleber Rosa
2020-09-25 13:15 ` Markus Armbruster
2020-09-25 13:24 ` Daniel P. Berrangé
2020-09-25 13:34 ` Eric Blake
2020-09-25 13:52 ` Markus Armbruster
2020-09-25 15:47 ` Eric Blake
2020-09-28 12:09 ` Markus Armbruster
2020-09-28 14:08 ` John Snow
2020-09-25 13:26 ` Eduardo Habkost
2020-09-25 16:33 ` John Snow
2020-09-24 16:01 ` Cleber Rosa
2020-09-22 21:00 ` [PATCH v2 29/38] qapi/gen.py: delint with pylint John Snow
2020-09-23 15:44 ` Eduardo Habkost
2020-09-23 18:38 ` John Snow
2020-09-24 18:44 ` Cleber Rosa
2020-09-22 21:00 ` [PATCH v2 30/38] qapi/introspect.py: Add a typed 'extra' structure John Snow
2020-09-23 16:13 ` Eduardo Habkost
2020-09-23 21:34 ` John Snow
2020-09-22 21:00 ` [PATCH v2 31/38] qapi/introspect.py: add _gen_features helper John Snow
2020-09-23 16:35 ` Eduardo Habkost
2020-09-23 21:43 ` John Snow
2020-09-23 21:54 ` Eduardo Habkost
2020-09-22 21:00 ` [PATCH v2 32/38] qapi/introspect.py: create a typed 'Node' data structure John Snow
2020-09-23 18:41 ` Eduardo Habkost
2020-09-23 21:48 ` John Snow
2020-09-23 22:44 ` John Snow
2020-09-22 21:00 ` [PATCH v2 33/38] qapi/introspect.py: add type hint annotations John Snow
2020-09-22 21:00 ` [PATCH v2 34/38] qapi/types.py: " John Snow
2020-09-23 19:11 ` Eduardo Habkost
2020-09-24 18:50 ` Cleber Rosa
2020-09-22 21:00 ` [PATCH v2 35/38] qapi/types.py: remove one-letter variables John Snow
2020-09-23 19:14 ` Eduardo Habkost
2020-09-23 22:11 ` John Snow
2020-09-24 20:54 ` Eduardo Habkost
2020-09-24 18:53 ` Cleber Rosa
2020-09-22 21:00 ` [PATCH v2 36/38] qapi/visit.py: assert tag_member contains a QAPISchemaEnumType John Snow
2020-09-23 19:15 ` Eduardo Habkost
2020-09-23 22:13 ` John Snow
2020-09-24 19:12 ` Cleber Rosa
2020-09-24 19:10 ` Cleber Rosa
2020-09-24 19:36 ` John Snow
2020-09-24 23:52 ` Cleber Rosa
2020-09-22 21:01 ` [PATCH v2 37/38] qapi/visit.py: remove unused parameters from gen_visit_object John Snow
2020-09-23 19:16 ` Eduardo Habkost
2020-09-24 19:13 ` Cleber Rosa
2020-09-22 21:01 ` [PATCH v2 38/38] qapi/visit.py: add type hint annotations John Snow
2020-09-23 19:17 ` Eduardo Habkost
2020-09-24 19:15 ` Cleber Rosa
2020-09-24 20:37 ` [PATCH v2 00/38] qapi: static typing conversion, pt1 John Snow
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200924192732.GC347036@localhost.localdomain \
--to=crosa@redhat.com \
--cc=alex.bennee@linaro.org \
--cc=armbru@redhat.com \
--cc=ehabkost@redhat.com \
--cc=jsnow@redhat.com \
--cc=mdroth@linux.vnet.ibm.com \
--cc=peter.maydell@linaro.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.