From: Markus Armbruster <armbru@redhat.com>
To: Eric Blake <eblake@redhat.com>
Cc: michael.roth@amd.com, jsnow@redhat.com,
Markus Armbruster <armbru@redhat.com>,
marcandre.lureau@redhat.com, qemu-devel@nongnu.org
Subject: Re: [PATCH 24/28] qapi: Enforce command naming rules
Date: Tue, 23 Mar 2021 22:19:56 +0100 [thread overview]
Message-ID: <87mtutlg8z.fsf@dusky.pond.sub.org> (raw)
In-Reply-To: <a6336cfa-6efb-8612-6b6b-018daa64a11f@redhat.com> (Eric Blake's message of "Tue, 23 Mar 2021 10:23:51 -0500")
Eric Blake <eblake@redhat.com> writes:
> On 3/23/21 4:40 AM, Markus Armbruster wrote:
>> Command names should be lower-case. Enforce this. Fix the fixable
>> offenders (all in tests/), and add the remainder to pragma
>> command-name-exceptions.
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>
>> +++ b/qapi/pragma.json
>> @@ -4,6 +4,24 @@
>> # add to them!
>> { 'pragma': {
>> # Commands allowed to return a non-dictionary:
>> + 'command-name-exceptions': [
>> + 'add_client',
>> + 'block_passwd',
>> + 'block_resize',
>> + 'block_set_io_throttle',
>> + 'client_migrate_info',
>> + 'device_add',
>> + 'device_del',
>> + 'expire_password',
>> + 'migrate_cancel',
>> + 'netdev_add',
>> + 'netdev_del',
>> + 'qmp_capabilities',
>> + 'set_link',
>> + 'set_password',
>> + 'system_powerdown',
>> + 'system_reset',
>> + 'system_wakeup' ],
>
> Outside the scope of this patch, do we have any intentions on adding
> alias commands or deprecating old spellings in favor of new ones?
>
> None of these have a capital letter...
>
> qmp_capabilities is probably the hardest one to get rid of, since you
> can't send any other commands until that one is complete (that is, you
> can't introspect if a replacement exists; if we add a new spelling, all
> you can do is try both spellings until one works, but that is extra
> traffic). The rest can be suitably probed via introspection.
Another option is to accept '-' instead of '_' in QMP command and
argument names.
The harder problem is QMP output.
>
>
>> +++ b/tests/unit/test-qmp-cmds.c
>> @@ -13,7 +13,7 @@
>>
>> static QmpCommandList qmp_commands;
>>
>> -UserDefThree *qmp_TestCmdReturnDefThree(Error **errp)
>> +UserDefThree *qmp_test_cmd_return_def_three(Error **errp)
>
> ...oh, we had a test command with capitals....
>
>> +++ b/scripts/qapi/expr.py
>> @@ -70,8 +70,9 @@ def check_defn_name_str(name, info, meta):
>> if meta == 'event':
>> check_name_upper(name, info, meta)
>> elif meta == 'command':
>> - check_name_lower(name, info, meta,
>> - permit_upper=True, permit_underscore=True)
>> + check_name_lower(
>> + name, info, meta,
>> + permit_underscore=name in info.pragma.command_name_exceptions)
>
> ...and earlier in the series, I had asked why you wanted
> permit_upper=True here. So it is now obvious that it was just for the
> tests and that you deferred fixing the tests until now. If you don't
> want to refactor the series, then it's at least worth a tweak to that
> commit message to call it out. At any rate, I'm glad to see the
> permit_upper=True gone!
>
> Reviewed-by: Eric Blake <eblake@redhat.com>
Thanks!
next prev parent reply other threads:[~2021-03-23 21:24 UTC|newest]
Thread overview: 93+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-23 9:39 [PATCH 00/28] qapi: Enforce naming rules Markus Armbruster
2021-03-23 9:39 ` [PATCH 01/28] qapi/pragma: Tidy up after removal of deprecated commands Markus Armbruster
2021-03-23 12:50 ` John Snow
2021-03-23 9:39 ` [PATCH 02/28] tests/qapi-schema: Drop redundant flat-union-inline test Markus Armbruster
2021-03-23 12:54 ` John Snow
2021-03-23 9:40 ` [PATCH 03/28] tests/qapi-schema: Rework comments on longhand member definitions Markus Armbruster
2021-03-23 13:00 ` John Snow
2021-03-23 13:58 ` Eric Blake
2021-03-23 14:25 ` John Snow
2021-03-23 13:59 ` Eric Blake
2021-03-23 14:27 ` John Snow
2021-03-23 9:40 ` [PATCH 04/28] tests/qapi-schema: Belatedly update comment on alternate clash Markus Armbruster
2021-03-23 13:12 ` John Snow
2021-03-23 9:40 ` [PATCH 05/28] tests/qapi-schema: Drop TODO comment on simple unions Markus Armbruster
2021-03-23 13:16 ` John Snow
2021-03-23 9:40 ` [PATCH 06/28] tests/qapi-schema: Tweak to demonstrate buggy member name check Markus Armbruster
2021-03-23 13:20 ` John Snow
2021-03-23 15:44 ` Markus Armbruster
2021-03-23 17:09 ` John Snow
2021-03-23 20:42 ` Markus Armbruster
2021-03-23 9:40 ` [PATCH 07/28] qapi: Fix to reject optional members with reserved names Markus Armbruster
2021-03-23 13:27 ` John Snow
2021-03-23 15:50 ` Markus Armbruster
2021-03-23 9:40 ` [PATCH 08/28] qapi: Support flat unions tag values with leading digit Markus Armbruster
2021-03-23 14:11 ` Eric Blake
2021-03-23 14:49 ` John Snow
2021-03-23 16:18 ` Markus Armbruster
2021-03-23 21:07 ` Markus Armbruster
2021-03-23 9:40 ` [PATCH 09/28] qapi: Lift enum-specific code out of check_name_str() Markus Armbruster
2021-03-23 14:13 ` Eric Blake
2021-03-23 21:44 ` John Snow
2021-03-23 22:11 ` John Snow
2021-03-24 5:55 ` Markus Armbruster
2021-03-23 9:40 ` [PATCH 10/28] qapi: Rework name checking in preparation of stricter checking Markus Armbruster
2021-03-23 14:20 ` Eric Blake
2021-03-23 14:30 ` John Snow
2021-03-23 14:40 ` Eric Blake
2021-03-23 16:25 ` Markus Armbruster
2021-03-23 21:14 ` Markus Armbruster
2021-03-23 22:15 ` John Snow
2021-03-24 5:57 ` Markus Armbruster
2021-03-24 20:11 ` John Snow
2021-03-25 6:18 ` Markus Armbruster
2021-03-25 17:48 ` John Snow
2021-03-26 5:25 ` Markus Armbruster
2021-03-23 9:40 ` [PATCH 11/28] qapi: Move uppercase rejection to check_name_lower() Markus Armbruster
2021-03-23 14:29 ` Eric Blake
2021-03-23 22:21 ` John Snow
2021-03-23 9:40 ` [PATCH 12/28] qapi: Consistently permit any case in downstream prefixes Markus Armbruster
2021-03-23 14:30 ` Eric Blake
2021-03-23 22:26 ` John Snow
2021-03-23 9:40 ` [PATCH 13/28] qapi: Enforce event naming rules Markus Armbruster
2021-03-23 14:32 ` Eric Blake
2021-03-23 22:31 ` John Snow
2021-03-24 6:22 ` Markus Armbruster
2021-03-24 20:07 ` John Snow
2021-03-25 6:22 ` Markus Armbruster
2021-03-25 17:50 ` John Snow
2021-03-23 9:40 ` [PATCH 14/28] qapi: Enforce type " Markus Armbruster
2021-03-23 14:50 ` Eric Blake
2021-03-23 16:27 ` Markus Armbruster
2021-03-23 9:40 ` [PATCH 15/28] tests/qapi-schema: Rename redefined-builtin to redefined-predefined Markus Armbruster
2021-03-23 14:55 ` Eric Blake
2021-03-23 9:40 ` [PATCH 16/28] qapi: Factor out QAPISchemaParser._check_pragma_list_of_str() Markus Armbruster
2021-03-23 15:01 ` Eric Blake
2021-03-23 9:40 ` [PATCH 17/28] tests/qapi-schema: Rename pragma-*-crap to pragma-value-not-* Markus Armbruster
2021-03-23 15:02 ` Eric Blake
2021-03-23 9:40 ` [PATCH 18/28] tests/qapi-schema: Rename returns-whitelist to returns-bad-type Markus Armbruster
2021-03-23 15:06 ` Eric Blake
2021-03-23 9:40 ` [PATCH 19/28] qapi: Rename pragma *-whitelist to *-exceptions Markus Armbruster
2021-03-23 15:09 ` Eric Blake
2021-03-23 16:35 ` Markus Armbruster
2021-03-23 9:40 ` [PATCH 20/28] qapi/pragma: Streamline comments on member-name-exceptions Markus Armbruster
2021-03-23 15:10 ` Eric Blake
2021-03-23 9:40 ` [PATCH 21/28] tests-qmp-cmds: Drop unused and incorrect qmp_TestIfCmd() Markus Armbruster
2021-03-23 15:11 ` Eric Blake
2021-03-23 9:40 ` [PATCH 22/28] qapi: Prepare for rejecting underscore in command and member names Markus Armbruster
2021-03-23 15:15 ` Eric Blake
2021-03-23 9:40 ` [PATCH 23/28] qapi: Enforce feature naming rules Markus Armbruster
2021-03-23 15:16 ` Eric Blake
2021-03-23 9:40 ` [PATCH 24/28] qapi: Enforce command " Markus Armbruster
2021-03-23 15:23 ` Eric Blake
2021-03-23 21:19 ` Markus Armbruster [this message]
2021-03-23 9:40 ` [PATCH 25/28] tests/qapi-schema: Switch member name clash test to struct Markus Armbruster
2021-03-23 15:42 ` Eric Blake
2021-03-23 9:40 ` [PATCH 26/28] qapi: Enforce struct member naming rules Markus Armbruster
2021-03-23 15:46 ` Eric Blake
2021-03-23 21:23 ` Markus Armbruster
2021-03-23 9:40 ` [PATCH 27/28] qapi: Enforce enum " Markus Armbruster
2021-03-23 15:47 ` Eric Blake
2021-03-23 9:40 ` [PATCH 28/28] qapi: Enforce union and alternate branch " Markus Armbruster
2021-03-23 16:05 ` Eric Blake
2021-03-23 21:24 ` Markus Armbruster
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=87mtutlg8z.fsf@dusky.pond.sub.org \
--to=armbru@redhat.com \
--cc=eblake@redhat.com \
--cc=jsnow@redhat.com \
--cc=marcandre.lureau@redhat.com \
--cc=michael.roth@amd.com \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).