[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH RFC 06/21] qapi-gen: New common driver for code
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH RFC 06/21] qapi-gen: New common driver for code and doc generators |
Date: |
Tue, 6 Feb 2018 08:56:23 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 |
On 02/06/2018 01:45 AM, Markus Armbruster wrote:
-def main(argv):
- (input_file, output_dir, do_c, do_h, prefix, opts) = parse_command_line()
-
- blurb = '''
- * Schema-defined QAPI/QMP commands
-'''
-
+def gen_commands(schema, output_dir, prefix):
+ blurb = ' * Schema-defined QAPI/QMP commands'
Some churn on the definition of blurb; worth tidying this in the
introduction earlier in the series?
Doesn't seem worth a separate patch, but I can try to reshuffle things
to reduce churn.
Yeah, my question was more about the conversion between multiline
'''...''' to single-line '...'; why not just use the single-line
construct earlier in the series when introducing the blurb variable.
Introduced in PATCH 01:
-c_comment = '''
-/*
- * schema-defined QMP->QAPI command dispatch
+blurb = '''
+ * Schema-defined QAPI/QMP commands
*
* Copyright IBM, Corp. 2011
Multiline made sense here.
Shortened in PATCH 02:
blurb = '''
* Schema-defined QAPI/QMP commands
- *
- * Copyright IBM, Corp. 2011
- *
- * Authors:
- * Anthony Liguori <address@hidden>
'''
Where it is now a single line...
Variable eliminated there in PATCH 17:
- blurb = ' * Schema-defined QAPI types'
- self._genc = QAPIGenC(blurb, __doc__)
- self._genh = QAPIGenH(blurb, __doc__)
+ self._module = {}
+ self._add_module(None, ' * Built-in QAPI types')
Oh, I didn't even notice that!
I could delay the reformatting until PATCH 16, or do it in PATCH 02.
Feels like a wash to me, but if you have a preference...
I guess my preference is to reformat to single line in PATCH 02, then
patch 6 doesn't have to touch it, and patch 16 can still get rid of it.
But, as it disappears by the end of the series anyways, I'm also okay if
you don't bother (churn is not necessarily bad, if it is still easy to
review).
The commit message talks only about the QAPI/QMP schema. It's confusing
because our poor taste in file names creates ambiguity: does
qapi-schema.json refer to ./qapi-schema.json (i.e. the QAPI/QMP one),
qga/qapi-schema.json, or both?
Perhaps I should rename qapi-schema.json to qapi/schema.json.
I'd be in favor of such a change. We'd also rename qga/qapi-schema.json
to qga/schema.json?
The commit message needs a note along the lines of "same for
qga/qapi-schema.json".
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH RFC 06/21] qapi-gen: New common driver for code and doc generators, Marc-Andre Lureau, 2018/02/05
[Qemu-devel] [PATCH RFC 14/21] qapi: Generate in source order, Markus Armbruster, 2018/02/02
[Qemu-devel] [PATCH RFC 03/21] qapi: New classes QAPIGenC, QAPIGenH, QAPIGenDoc, Markus Armbruster, 2018/02/02