Re: [Qemu-devel] [PATCH v7 07/21] qapi: avoid interleaving sections and parameters

[Markus Armbruster]

Marc-André Lureau <address@hidden> writes:

> Follow documentation guideline, body, parameters then additional
> sections.

Commit message wording nit: we don't have a concept of "sections", yet.

I don't think the guidelines demand such an order at this point.  Here's
the applicable paragraph from docs/qapi-code-gen.txt:

    Comments are allowed; anything between an unquoted # and the following
    newline is ignored.  Although there is not yet a documentation
    generator, a form of stylized comments has developed for consistently
    documenting details about an expression and when it was added to the
    schema.  The documentation is delimited between two lines of ##, then
    the first line names the expression, an optional overview is provided,
    then individual documentation about each member of 'data' is provided,
    and finally, a 'Since: x.y.z' tag lists the release that introduced
    the expression.  Optional members are tagged with the phrase
    '#optional', often with their default value; and extensions added
    after the expression was first released are also given a '(since
    x.y.z)' comment.  For example:

A pedantical reader would conclude only "Since" can follow the "members
of 'data'".

I'm feeling less pedantical, and conclude that additional information
not mentioned in this paragraph can be stuck wherever it reads well.
Matches existing practice.

That said, I don't mind moving stuff around if it helps the doc
generator later on, and doesn't make things less readable.

> Signed-off-by: Marc-André Lureau <address@hidden>
> ---
>  qapi-schema.json     | 10 +++++-----
>  qapi/event.json      |  4 ++--
>  qapi/introspect.json |  3 +--
>  3 files changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/qapi-schema.json b/qapi-schema.json
> index ac6352fcf9..10a3506528 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -4729,17 +4729,17 @@
>  # it should be passed by management with device_add command when
>  # a CPU is being hotplugged.
>  #
> +# @node-id: #optional NUMA node ID the CPU belongs to
> +# @socket-id: #optional socket number within node/board the CPU belongs to
> +# @core-id: #optional core number within socket the CPU belongs to
> +# @thread-id: #optional thread number within core the CPU belongs to
> +#
>  # Note: currently there are 4 properties that could be present
>  # but management should be prepared to pass through other
>  # properties with device_add command to allow for future
>  # interface extension. This also requires the filed names to be kept in
>  # sync with the properties passed to -device/device_add.
>  #
> -# @node-id: #optional NUMA node ID the CPU belongs to
> -# @socket-id: #optional socket number within node/board the CPU belongs to
> -# @core-id: #optional core number within socket the CPU belongs to
> -# @thread-id: #optional thread number within core the CPU belongs to
> -#
>  # Since: 2.7
>  ##
>  { 'struct': 'CpuInstanceProperties',

No objection.

> diff --git a/qapi/event.json b/qapi/event.json
> index 37bf34ed6d..c7689b211d 100644
> --- a/qapi/event.json
> +++ b/qapi/event.json
> @@ -272,9 +272,9 @@
>  #
>  # Emitted when guest executes ACPI _OST method.
>  #
> -# Since: 2.1
> -#
>  # @info: ACPIOSTInfo type as described in qapi-schema.json
> +#
> +# Since: 2.1
>  ##
>  { 'event': 'ACPI_DEVICE_OST',
>       'data': { 'info': 'ACPIOSTInfo' } }

Consistency improvement.

> diff --git a/qapi/introspect.json b/qapi/introspect.json
> index 464097a235..f6adc439bb 100644
> --- a/qapi/introspect.json
> +++ b/qapi/introspect.json
> @@ -78,14 +78,13 @@
>  # @SchemaInfo:
>  #
>  # @name: the entity's name, inherited from @base.
> +#        The SchemaInfo is always referenced by this name.
>  #        Commands and events have the name defined in the QAPI schema.
>  #        Unlike command and event names, type names are not part of
>  #        the wire ABI.  Consequently, type names are meaningless
>  #        strings here, although they are still guaranteed unique
>  #        regardless of @meta-type.
>  #
> -# All references to other SchemaInfo are by name.
> -#
>  # @meta-type: the entity's meta type, inherited from @base.
>  #
>  # Additional members depend on the value of @meta-type.

Okay.

PATCH 02 "qapi: move QKeyCode doc body at the top" could be squashed
into this one, I think.

Overall, no objections, except the commit message's first line
forward-references the "sections" concept, and its rationale is a bit
weak.

What about this:

    qapi: Reorder doc comments for future doc generator

    The doc generator we're going to add expects a fairly rigid doc
    comment structure.  Reorder / rephrase some to please it.

To be clear: I still doubt this rigid order is a good idea.  But as long
as I can't point to actual harm, I'm willing to accept it, in the
interest of getting stuff done.