Re: [PATCH 03/21] qapi/migration: Don't document MigrationParameter

[Fabiano Rosas <farosas@suse.de>]

Markus Armbruster <armbru@redhat.com> writes:

> Markus Armbruster <armbru@redhat.com> writes:
>
>> Fabiano Rosas <farosas@suse.de> writes:
>>
>>> The MigrationParameter (singular) enumeration is not part of the
>>> migration QMP API, it's only used for nicely converting HMP strings
>>> into MigrationParameters (plural) members and for providing readline
>>> completion.
>>
>>
>>
>>> Documenting this enum only serves to duplicate documentation between
>>> MigrationParameter and MigrationParameters.
>>>
>>> Add an exception to QAPIs pragma.json and stop documenting it.
>>>
>>> Signed-off-by: Fabiano Rosas <farosas@suse.de>
>>> ---
>>>  qapi/migration.json | 152 +-------------------------------------------
>>>  qapi/pragma.json    |   3 +-
>>>  2 files changed, 3 insertions(+), 152 deletions(-)
>>>
>>> diff --git a/qapi/migration.json b/qapi/migration.json
>>> index 080968993a..452e6dedaa 100644
>>> --- a/qapi/migration.json
>>> +++ b/qapi/migration.json
>>> @@ -734,157 +734,7 @@
>>>  ##
>>>  # @MigrationParameter:
>>>  #
>>> -# Migration parameters enumeration
>>> -#
>>> -# @announce-initial: Initial delay (in milliseconds) before sending
>>> -#     the first announce (Since 4.0)
>>
>> [...]
>>
>>> -# @direct-io: Open migration files with O_DIRECT when possible.  This
>>> -#     only has effect if the @mapped-ram capability is enabled.
>>> -#     (Since 9.1)
>>> +# Migration parameters enumeration. See @MigrationParameters for more info.
>>
>> Suggest something like
>>
>>    # The enumeration values mirror the members of MigrationParameters,
>>    # which see.
>>
>> I could compare the deleted docs with MigrationParameters docs, but I
>> doubt it's worthwhile: the type is only ever used internally.  That it
>> appears in the QEMU QMP Reference Manual anyway is a defect.  There are
>> quite a few more like it (list appended).
>>
>> If I remember correctly, John Snow's doc generator work will fix this
>> defect.
>
> Until then, this patch has a drawback: the manual now shows all the
> members as "Not documented" .  Ugly and a bit embarrassing.  Maybe even
> confusing.
>

I expect the part about checking @MigrationParameters to be clear enough
in explaining why the members are "Not documented".

I could live with the enum being documented still, but there's always
churn when people touch migration.json and get something wrong due to
the duplication. I'd rather not have it.