Re: [PATCH 00/11] docs/qapi: revise and expand QAPI documentation spec

[Markus Armbruster <armbru@redhat.com>]

John Snow <jsnow@redhat.com> writes:

> This patchset looks at expanding and clarifying the documentation for
> ... writing documentation. Primarily, the doc is being organized into
> subsections - one for each QAPI doc section. These sections are
> re-ordered to be documented in the same order as they are expected to
> appear in-usage.

While I appreciate the split into single-purpose patches, I still need
to review the entire doc update as a whole.

   Definition documentation
   ~~~~~~~~~~~~~~~~~~~~~~~~

   Definition documentation, if present, must immediately precede the
   definition it documents.

   When documentation is required (see pragma_ 'doc-required'), every
   definition must have documentation.

   Definition documentation starts with a description naming the
  -definition with an optional indented overview, a description of each
  -argument (for commands and events), member (for structs and unions),
  -branch (for alternates), or value (for enums), a description of each
  -feature (if any), and finally optional tagged sections.
  +definition with an optional indented overview (the :ref:`doc-intro`),
  +:ref:`descriptions <doc-descrip>` of each argument (for commands and
  +events), member (for structs and unions), branch (for alternates), or
  +value (for enums), :ref:`feature descriptions <doc-feat>` of each
  +feature (if any), and finally optional :ref:`tagged sections
  +<doc-tagged>`.

This needs work.  Copying from my review of PATCH 07...  The order of
tagged sections is left unspecified, which is fine now, but becomes a
gap when your "qapi: enforce section ordering" series lands.  "Details"
sections aren't mentioned at all.

Try a list?  Or do away with the overview entirely?  You do describe the
definition documentation contents in the required order below, which
could be enough.

  -Descriptions start with '\@name:'.  The description text must be
  -indented like this::
  +
  +.. _doc-intro:
  +
  +Intro
  +^^^^^
  +
  +The Intro section names the symbol being defined and is optionally
  +followed by an indented brief summary or overview of the definition.
  +
  +The intro section *may* consist of multiple indented paragraphs, but
  +is ideally a single summary line that does not repeat type
  +information or reference individual arguments/members/et al::

et al.

Perhaps "does not repeat type information and does not contain forward
references, say to command arguments".

  +
  +  ##
  +  # @query-block-jobs:
  +  #     Return information about long-running block device operations.
  +  #
  +
  +
  +.. _doc-descrip:
  +
  +Descriptions
  +^^^^^^^^^^^^

Would be nice to have a better term than "descriptions".  "Argument /
member / branch / value descriptions" is rather long.

  +
  +Descriptions start with '\@name:'.  These are used to document
  +arguments (for commands and events), members (for structs and unions),
  +branches (for alternates), or values (for enums).
  +
  +The description text must be indented like this::

    # @name: Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed
    #     do eiusmod tempor incididunt ut labore et dolore magna aliqua.

   .. FIXME The parser accepts these things in almost any order.

   .. FIXME union branches should be described, too.

   Extensions added after the definition was first released carry a
  -"(since x.y.z)" comment.
  +"(since x.y)" comment.

  -The feature descriptions must be preceded by a blank line and then a
  -line "Features:", like this::

  -  #
  -  # Features:
  -  #
  -  # @feature: Description text
  +.. _doc-tagged:
  +
  +Tagged sections
  +^^^^^^^^^^^^^^^

   A tagged section begins with a paragraph that starts with one of the
   following words: "Since:", "Returns:", "Errors:", "TODO:".  It ends with
   the start of a new section.

   The second and subsequent lines of tagged sections must be indented
   like this::

    # TODO: Ut enim ad minim veniam, quis nostrud exercitation ullamco
    #     laboris nisi ut aliquip ex ea commodo consequat.
    #
    #     Duis aute irure dolor in reprehenderit in voluptate velit esse
    #     cillum dolore eu fugiat nulla pariatur.

This factors out the common syntax (good) at the price of breaking the
linear sequence of sections (not so good).  Hmm.

  -"Returns" and "Errors" sections are only valid for commands.  They
  -document the success and the error response, respectively.
  +
  +Returns
  +^^^^^^^
  +
  +"Returns" :ref:`doc-tagged` are only valid for commands.  They document the

This comes out as

    “Returns” Tagged sections are only valid for commands. They document the

"Tagged" should not be capitalized.

  +success response of the command. If there is some nuance to capture
  +with the return value, it may be documented as::
  +
  +  #
  +  # Returns: a list describing each virtual block device.  Filter nodes
  +  #     that were created implicitly are skipped over.
  +  #
  +
  +Long descriptions are written as a block, with additional lines indented.

Redundant with the common syntax under "Tagged sections".  We need to
make up our mind whether to factor out syntax or not.

  +
  +The type of the return value will be added to the documentation
  +automatically. If there is nothing relevant to add apart from type,
  +this section may be omitted.

Maybe do this first, and only then a suitable rephrasing of "If there is
some nuance ..."

  +
  +
  +Errors
  +^^^^^^
  +
  +"Errors" :ref:`doc-tagged` are only valid for commands.  They document
  +the error responses.

   "Errors" sections should be formatted as an rST list, each entry
   detailing a relevant error condition.  For example::

    # Errors:
    #     - If @device does not exist, DeviceNotFound
    #     - Any other error returns a GenericError.

Problematic example copied from block-commit.  Tidied up there in commit
b3bd7617dba.  Better steal something else.

Hmm, many existing Errors: sections don't mention the error class.  E.g.
nbd-server-add:

    # Errors:
    #     - if the server is not running
    #     - if an export with the same name already exists

Should we document this as acceptable?

  -A "Since: x.y.z" tagged section lists the release that introduced the
  -definition.

  -"TODO" sections are not rendered (they are for developers, not users of
  -QMP).  In other sections, the text is formatted, and rST markup can be
  -used.
  +.. _doc-feat:
  +
  +Feature Descriptions
  +^^^^^^^^^^^^^^^^^^^^
  +
  +Feature descriptions are a :ref:`doc-descrip` section that document features
  +that apply to this definition.
  +
  +The feature descriptions must be preceded by a blank line and then a
  +line "Features:", like this::
  +
  +  #
  +  # Features:
  +  #
  +  # @feature: Description text

Perhaps something like

   Feature descriptions document the features that apply to this
   definition.  They start with a line "Features:" followed by one
   description per feature in the same syntax as argument / member /
   branch / value descriptions, like this::

  +
  +
  +Details
  +^^^^^^^
  +
  +Details sections are comprised of one or more paragraphs of rST
  +formatted text, but otherwise are not marked up with any QAPI-specific
  +syntax. In general, this text contains in-depth explanations of the
  +definition being documented and may include various rST admonitions
  +like ``.. seealso::``, ``.. note::``, etc. This is also the section
  +where QMP usage examples should be placed.

Is the section concept helpful here?

Internally, it's a single section unless split by TODO sections.
Whether and how it's split shouldn't affect the output at all.

Users don't care for all that.  For them, it's just paragraphs.

   QMP Examples can be added by using the ``.. qmp-example::`` directive.
   In its simplest form, this can be used to contain a single QMP code
   block which accepts standard JSON syntax with additional server
   directionality indicators (``->`` and ``<-``), and elisions.  An
   elision is commonly ``...``, but it can also be or a pair of ``...``
   with text in between.

   Optionally, a plaintext title may be provided by using the ``:title:``
   directive option.  If the title is omitted, the example title will
   default to "Example:".

   A simple QMP example::

     # .. qmp-example::
     #
     #     -> { "execute": "query-name" }
     #     <- { "return": { "name": "Fred" } }

   More complex or multi-step examples where exposition is needed before
   or between QMP code blocks can be created by using the ``:annotated:``
   directive option.  When using this option, nested QMP code blocks must
   be entered explicitly with rST's ``::`` syntax.

   For example::

     # .. qmp-example::
     #    :annotated:
     #    :title: A more complex demonstration
     #
     #    This is a more complex example that can use
     #    ``arbitrary rST syntax`` in its exposition::
     #
     #     -> { "execute": "query-block" }
     #     <- { "return": [
     #             {
     #               "device": "ide0-hd0",
     #               ...
     #             }
     #             ... more ...
     #          ] }
     #
     #    Above, lengthy output has been omitted for brevity.

   Highlighting in non-QMP languages can be accomplished by using the
   ``.. code-block:: lang`` directive, and non-highlighted text can be
   achieved by omitting the language argument.


  +Since
  +^^^^^
  +
  +A "Since: x.y" :ref:`doc-tagged` lists the release that introduced the
  +definition::
  +
  +  #
  +  # Since: 11.2
  +  #
  +
  +
  +TODO
  +^^^^
  +
  +"TODO" tagged sections are not rendered (they are for developers, not users of
  +QMP). They can appear anywhere after the introductory section::
  +
  +  #
  +  ## TODO: This is not rendered in the documentation.
  +  #
  +
  +
  +Putting it all together
  +^^^^^^^^^^^^^^^^^^^^^^^
  +
   Examples of complete definition documentation::

    ##
    # @BlockStats:
    #     Statistics of a virtual block device or a block backing device.
    #
    # @device: If the stats are for a virtual block device, the name
    #     corresponding to the virtual block device.
    #
    # @node-name: The node name of the device.  (Since 2.3)
    #
    # ... more members ...
    #
    # Since: 0.14
    ##
    { 'struct': 'BlockStats',
      'data': {'*device': 'str', '*node-name': 'str',
               ... more members ... } }

    ##
    # @query-blockstats:
    #     Query the @BlockStats for all virtual block devices.
    #
    # @query-nodes: If true, the command will query all the block nodes
    #     ... explain, explain ...
    #     (Since 2.3)
    #
    # Returns: A list of @BlockStats for each virtual block devices.
    #
    # Since: 0.14
    #
    # .. qmp-example::
    #
    #     -> { "execute": "query-blockstats" }
    #     <- {
    #          ...
    #        }
    ##
    { 'command': 'query-blockstats',
      'data': { '*query-nodes': 'bool' },
      'returns': ['BlockStats'] }


   Markup pitfalls
   ~~~~~~~~~~~~~~~