Re: [PATCH 6/7] qapi: Drop unnecessary horizontal spacing in comments

[Markus Armbruster]

Andrea Bolognani <abologna@redhat.com> writes:

> On Mon, May 02, 2022 at 10:50:07AM +0200, Markus Armbruster wrote:
>> Andrea Bolognani <abologna@redhat.com> writes:
>> > -# @writeback:   true if writeback mode is enabled
>> > -# @direct:      true if the host page cache is bypassed (O_DIRECT)
>> > -# @no-flush:    true if flush requests are ignored for the device
>> > +# @writeback: true if writeback mode is enabled
>> > +# @direct:    true if the host page cache is bypassed (O_DIRECT)
>> > +# @no-flush:  true if flush requests are ignored for the device
>>
>> I'm no fan of horizontally aligning descriptions, because when you add a
>> longer name, you either realign (I hate the churn) or live with the
>> inconsistency (I hate that, too).
>
> We seem to be in violent agreement on the topic, but it's apparent
> that other people feel diffently :)
>
>> I doubt changing to a different alignment now is useful.  The next
>> patch, however, drops the alignment entirely.  Possibly useful.
>>
>> Thoughts?
>
> My rationale for splitting things the way I did is that, if dropping
> the horizontal alignment entirely was not considered desirable, we
> could at least get rid of the extra whitespace.

Understood.

>                                                 But if you think that
> the benefit from the half measure doesn't offset the cost of the
> churn it causes, I'm happy to drop these hunks and go straight from
> the current status to no horizontal alignment at all in one fell
> swoop with the next patch.

Show us the patches, and then we can decide whether the improvement is
worth the churn.

>> > -# Since:  0.14
>> > +# Since: 0.14
>>
>> This one is TAG: TEXT, whereas the one above is a multiple @NAME:
>> DESCRIPTION.  Extra space in the latter can provide alignment.  Extra
>> space in the former is always redundant.  I'd take a patch dropping
>> these obviously redundant spaces without debate :)
>
> Okay, I'll respin this so that the first patch drops all extra
> whitespace in contexts where horizontal alignment is either not
> attempted or not possible, and the second one implements the more
> controversial changes.

The first one is another no-brainer.