All of lore.kernel.org
 help / color / mirror / Atom feed
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 10/38] qapi/common.py: delint with pylint
Date: Thu, 24 Sep 2020 15:30:51 -0400	[thread overview]
Message-ID: <20200924193051.GE347036@localhost.localdomain> (raw)
In-Reply-To: <cc0047ca-cb29-67fa-759d-500d226e928c@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 2218 bytes --]

On Wed, Sep 23, 2020 at 01:37:06PM -0400, John Snow wrote:
> On 9/23/20 12:01 PM, Cleber Rosa wrote:
> > On Tue, Sep 22, 2020 at 05:00:33PM -0400, John Snow wrote:
> > > At this point, that just means using a consistent strategy for constant names.
> > > constants get UPPER_CASE and names not used externally get a leading underscore.
> > > 
> > > As a preference, while renaming constants to be UPPERCASE, move them to
> > > the head of the file. Generally, it's nice to be able to audit the code
> > > that runs on import in one central place.
> > > 
> > > Signed-off-by: John Snow <jsnow@redhat.com>
> > > ---
> > >   scripts/qapi/common.py | 18 ++++++++----------
> > >   scripts/qapi/schema.py | 14 +++++++-------
> > >   2 files changed, 15 insertions(+), 17 deletions(-)
> > > 
> > > diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
> > > index e0c5871b10..bddfb5a9e5 100644
> > > --- a/scripts/qapi/common.py
> > > +++ b/scripts/qapi/common.py
> > > @@ -14,6 +14,11 @@
> > >   import re
> > > +EATSPACE = '\033EATSPACE.'
> > > +POINTER_SUFFIX = ' *' + EATSPACE
> > > +_C_NAME_TRANS = str.maketrans('.-', '__')
> > 
> > IMO _C_NAME_TRANS is solely the concern of the c_name() function, and
> > should not be a global.  If you're concerned with speed (which I don't
> > think you should) you could still do:
> > 
> 
> It's true, but that's why it's marked internal here with the leading
> underscore -- it will not be exported. It was also *already* defined at the
> module level, I didn't hoist it.
> 
> I think what is written here is already the simplest thing that works.
> 
> >     def c_name(name, protect=True,
> >                name_translation=str.maketrans('.-', '__')):
> >        ...
> >        name = name.translate(name_translation)
> > 
> > Keeping in mind that you're adding a mutable type to a function
> > argument *on purpose*.  I'd really favor having that statement within
> > the only function that uses it, though.
> > 
> 
> That complicates the signature, so I think we shouldn't.
> 
> --js
> 
> 

Not a too strong opinion about this to block it.  So if I hadn't
already:

Reviewed-by: Cleber Rosa <crosa@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2020-09-24 19:34 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
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 [this message]
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=20200924193051.GE347036@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.