Peter Xu <pet...@redhat.com> writes:

> On Wed, Jul 26, 2023 at 08:21:35AM +0200, Markus Armbruster wrote:
>> Peter Xu <pet...@redhat.com> writes:
>> 
>> > Hi, Markus,
>> >
>> > On Tue, Jul 25, 2023 at 01:10:01PM +0200, Markus Armbruster wrote:
>> 
>> [...]
>> 
>> >> For better or worse, we duplicate full documentation between
>> >> MigrationParameter, MigrateSetParameters, and MigrationParameters.  This
>> >> would be the first instance where we reference instead.  I'm not opposed
>> >> to use references, but if we do, I want them used consistently.
>> >
>> > We discussed this over the other "switchover" parameter, but that patchset
>> > just stranded..
>> >
>> > Perhaps I just provide a pre-requisite patch to remove all the comments in
>> > MigrateSetParameters and MigrationParameters, letting them all point to
>> > MigrationParameter?
>> 
>> Simplifies maintaining the doc commments.  But how does it affect the
>> documentation generated from it?  Better, neutral, or worse?
>
> Probably somewhere neutral.  There are definitely benefits, shorter
> man/html page in total, and avoid accidentally different docs over the same
> fields.  E.g., we sometimes have different wordings for different objects:
>
>        max-cpu-throttle
>               maximum cpu throttle percentage.  Defaults to 99.  (Since 3.1)
>
>        max-cpu-throttle: int (optional)
>               maximum cpu throttle percentage.  The default value is 99. 
> (Since 3.1)
>
> This one is fine, but it's just very easy to leak in something that shows
> differently.  It's good to provide coherent documentation for the same
> fields over all three objects.

Yes, but we've been doing okay regardless.

The drawback of replacing duplicates by references is that readers need
to follow the references.

Less onerous when the references can be clicked.

If we de-duplicate, which copy do we keep, MigrationParameter,
MigrateSetParameters, or MigrationParameter?  Do we have an idea which
of these users are likely to read first?

> When looking at qemu-qmp-ref.7, it can be like this when we can invite the
> reader to read the other section (assuming we only keep MigrationParameter
> to keep the documentations):
>
>    MigrationParameters (Object)
>
>        The object structure to represent a list of migration parameters.
>        The optional members aren't actually optional.  For detailed
>        explanation for each of the field, please refer to the documentation
>        of MigrationParameter.
>
> But the problem is we always will generate the Members entry, where now
> it'll all filled up with "Not documented"..
>
>    Members
>        announce-initial: int (optional)
>               Not documented
>
>        announce-max: int (optional)
>               Not documented
>
>        announce-rounds: int (optional)
>               Not documented
>
>        [...]
>
> I think maybe it's better we just list the members without showing "Not
> documented" every time for the other two objects.  Not sure whether it's an
> easy way to fix it, or is it a major issue.

The automatic generation of "Not documented" documentation is a
stop-gap.  Leaving a member undocumented should be a hard error.  It
isn't only because we have 511 instances to fix.

Perhaps a directive to ignore undocumented members could be useful.
I.e. to suppress the automatic "Not documented" documented now, the
error later.

We could write things out in longhand instead, like

    # @announce-initial: Same as MigrationParameter member
    #     @announce-initial.

> For developers, dedup the comment should always be a win, afaict.

No argument.

Reply via email to