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

> On Fri, Aug 04, 2023 at 02:06:02PM +0200, Markus Armbruster wrote:
>> 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?
>
> I chose MigrationParameter for no explicit reason, because I can't find
> good argument to differenciate them.  Please let me know if you have any
> suggestion.
>
>> 
>> > 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.
>
> Yes I can definitely do this.
>
> Since I don't really know whether the "put a link" will work at all (at
> least man page doesn't really have those, does it?), would this be the way
> you suggest us forward?

I don't remember offhand how the QAPI doc generation machinery adds
links.  I can look it up for you after my vacation (two weeks, starting
basically now).

> Note that I am also always happy to simply duplicate the three paragraphs
> just like before; that's not something I must do with solving the migration
> problem so far, we can decouple the two problems essentially.  But since
> we're at it, if you think worthwhile we may have a chance get rid of
> duplicated documents here (before code) I can try.
>
>> 
>> > For developers, dedup the comment should always be a win, afaict.
>> 
>> No argument.
>
> Let me explain a bit: I meant the patch author who will reduce writting
> duplicated documents, making sure everything match together.  And reviewers
> who will read the duplicated content, making sure that everything match
> together again.  The two efforts can be avoided.  That's all I meant here
> for when I was referring to as "developers" in this context..  Not everyone
> as a common sense of developer.

By "no argument", I mean we don't need to argue about this.  I actually
agree with you that de-duplication would be net positive for developers.


Reply via email to