On Fri, Mar 28, 2025 at 4:36 AM Markus Armbruster <arm...@redhat.com> wrote:
> John Snow <js...@redhat.com> writes:
>
> > Well, I tried. Maybe not very hard. Sorry!
>
> No need to be sorry! Kick-starting discussion with limited effort is
> better than a big effort going into a direction that turns out to be
> unwanted once we discuss it.
>
> Instead of just rephrasing Returns descriptions, I'd like us to consider
> both Returns and intro. See below for why.
>
> > Signed-off-by: John Snow <js...@redhat.com>
> > ---
> > qapi/block-core.json | 6 +++---
> > qapi/block-export.json | 2 +-
> > qapi/block.json | 2 +-
> > qapi/control.json | 5 ++---
> > qapi/dump.json | 5 ++---
> > qapi/introspect.json | 6 +++---
> > qapi/job.json | 2 +-
> > qapi/machine-target.json | 7 +++----
> > qapi/misc-target.json | 2 +-
> > qapi/misc.json | 5 ++---
> > qapi/net.json | 2 +-
> > qapi/pci.json | 2 +-
> > qapi/qdev.json | 3 +--
> > qapi/qom.json | 8 +++-----
> > qapi/stats.json | 2 +-
> > qapi/trace.json | 2 +-
> > qapi/ui.json | 2 +-
> > qapi/virtio.json | 6 +++---
> > 18 files changed, 31 insertions(+), 38 deletions(-)
> >
> > diff --git a/qapi/block-core.json b/qapi/block-core.json
> > index 92b2e368b72..eb97b70cd80 100644
> > --- a/qapi/block-core.json
> > +++ b/qapi/block-core.json
> > @@ -763,7 +763,7 @@
> ##
> # @query-block:
> > #
> > # Get a list of BlockInfo for all virtual block devices.
> > #
> > -# Returns: a list of @BlockInfo describing each virtual block device.
> > +# Returns: a list describing each virtual block device.
> > # Filter nodes that were created implicitly are skipped over.
> > #
> > # Since: 0.14
> > @@ -1168,7 +1168,7 @@
> ##
> # @query-blockstats:
> #
> # Query the @BlockStats for all virtual block devices.
> #
> # @query-nodes: If true, the command will query all the block nodes
> # that have a node name, in a list which will include "parent"
> # information, but not "backing". If false or omitted, the
> # behavior is as before - query all the device backends,
> # recursively including their "parent" and "backing". Filter
> > # nodes that were created implicitly are skipped over in this
> > # mode. (Since 2.3)
> > #
> > -# Returns: A list of @BlockStats for each virtual block devices.
> > +# Returns: A list of statistics for each virtual block device.
> > #
> > # Since: 0.14
> > #
> > @@ -1440,7 +1440,7 @@
> ##
> # @query-block-jobs:
> > #
> > # Return information about long-running block device operations.
> > #
> > -# Returns: a list of @BlockJobInfo for each active block job
> > +# Returns: a list of job info for each active block job
> > #
> > # Since: 1.1
> > ##
>
> Stopping here, because I believe this is already enough to make a useful
> observation.
>
> There's overlap between query-FOO's intro and Returns. The intro
> describes the command's purpose, and the purpose of a query-FOO is to
> return certain information. Returns describes the value returned. Both
> talk about the same thing, namely the value returned.
>
> Why two prose descriptions of the value returned? Wouldn't one be
> better?
>
> Let's try. We need to choose where to put it, intro or Returns.
>
> When Returns comes right after intro, putting it into Returns can work
> fine, because the command's purpose still comes first. For instance:
>
> ##
> # @query-block:
> #
> # Return information about all virtual block devices. Filter nodes
> # that were created implicitly are skipped over.
>
> renders like
>
> Command query-block (Since: 0.14)
>
> Return information about all virtual block devices. Filter nodes
> that were created implicitly are skipped over.
>
> Return:
> ["BlockInfo"]
>
> and
>
> ##
> # @query-block:
> #
> # Returns: Information about all virtual block devices.
> # Filter nodes that were created implicitly are skipped over.
>
> renders like
>
> Command query-block (Since: 0.14)
>
> Return:
> ["BlockInfo"] -- Information about all virtual block
> devices. Filter nodes that were created implicitly are skipped
> over.
>
> Both work.
>
> But when there's something in between, intro is preferable. For
> instance:
>
> ##
> # @query-blockstats:
> #
> # Return statistics for all virtual block devices.
> #
> # @query-nodes: If true, the command will query all the block nodes
> # that have a node name, in a list which will include "parent"
> # information, but not "backing". If false or omitted, the
> # behavior is as before - query all the device backends,
> # recursively including their "parent" and "backing". Filter
> # nodes that were created implicitly are skipped over in this
> # mode. (Since 2.3)
>
> renders like
>
> Command query-blockstats (Since: 0.14)
>
> Return statistics for all virtual block devices.
>
> Arguments:
> * **query-nodes** ("boolean", *optional*) -- If true, the
> command will query all the block nodes that have a node name,
> in a list which will include "parent" information, but not
> "backing". If false or omitted, the behavior is as before -
> query all the device backends, recursively including their
> "parent" and "backing". Filter nodes that were created
> implicitly are skipped over in this mode. (Since 2.3)
>
> Return:
> ["BlockStats"]
>
> while
>
> ##
> # @query-blockstats:
> #
> # @query-nodes: If true, the command will query all the block nodes
> # that have a node name, in a list which will include "parent"
> # information, but not "backing". If false or omitted, the
> # behavior is as before - query all the device backends,
> # recursively including their "parent" and "backing". Filter
> # nodes that were created implicitly are skipped over in this
> # mode. (Since 2.3)
> #
> # Returns: Statistics for all virtual block devices
>
> renders like
>
> Command query-blockstats (Since: 0.14)
>
> Arguments:
> * **query-nodes** ("boolean", *optional*) -- If true, the
> command will query all the block nodes that have a node name,
> in a list which will include "parent" information, but not
> "backing". If false or omitted, the behavior is as before -
> query all the device backends, recursively including their
> "parent" and "backing". Filter nodes that were created
> implicitly are skipped over in this mode. (Since 2.3)
>
> Return:
> ["BlockStats"] -- Statistics for all virtual block devices
>
> I much prefer the former, because it puts the command's purpose where it
> belongs: first.
>
> Perhaps we should simply use the intro always.
>
I think for cases where the doc block is short and we have a desire to
merge "returns" and "intro", the intro makes the most sense if there isn't
anything of particular value assigned to the return value to begin with.
So, more or less, yeah: if semantics are partially duplicated between
intro/returns, I'm in favor of putting it all in the intro and allowing
transmogrifier generate the return type info.
I don't think there's a good case to make for a doc block with no intro but
a healthy paragraph in the returns section, that looks goofy.
Of course: I think there are still circumstances where we'll want both the
intro and the returns info explicitly labeled, when we have some
information to document about the semantics of that return value.
>
> Thoughts?
>
>
