Re: [PATCH 0/1] xl.cfg man page cleanup and fixes

[Armando Vega <armando@greenhost.nl>]

On 06/06/2017 12:28 PM, Ian Jackson wrote:
> Armando Vega writes ("[PATCH 0/1] xl.cfg man page cleanup and fixes"):
>> so I've made a new round of cleaning and fixing. There was quite some work to
>> be done with this one. And there are a few issues that are left still, but
>> at least not with the general correctness and style of the manual. More info
>> below.
> 
> Thanks for your excellent work.
> 
> Thanks also for your clarity in this message:
> 
>> I've had to rework the NUMA node examples as it had what I would call a
>> counting error and in the end presented incorrect information. It would be
>> great if someone could check me up on that once more. Also, there is no clear
>> explanation whether a person can use ^nodes:1 and nodes:^1 interchangably and
>> to be honest I wasn't sure myself. Haven't had the time to give this proper
>> testing.
> 
> I think this means we should get a review from Dario or George (added
> to the To: field).
> 
>> Also there is an issue with at least one of the HVM-only options
>> that can actually be used with PV guests as well. I know because
>> we've been using CPU masking / feature leveling for our PV guests on
>> Xen 4.6., and if it went from being HVM-only to also on PV I
>> couldn't say when that actually happened.  It is quite possible that
>> there are more such options which aren't exclusive to one type
>> anymore.
>>
>> Anyway, that is something to be discussed and fixed in another iteration.
> 
> Fair enough.
> 
> 
> I have a couple of suggestions for improvement to your approach:
> 
> Some of the information you have put in this 0/1 cover letter could
> usefully be in the commit message for the patch, instead.  In
> particular, the commit message ought to explain that you are making a
> semantic change and why.  If you need to respin this patch, please do
> that.
> 

I would've thought of that myself if I weren't so exhausted yesterday when I
finished the patch and was writing the commit message. I'll make the commit
message more verbose and resend the new patch. I will however wait for George
and Dario to weigh in first in case I need to do more editing on that front, so
as to avoid unnecessary iterations.

> 
> The other point is: did you see my comment about semantic linefeeds ?
> I wrote:
> 
>    Rather, if a line gets too long, break it at a phrase or sentence
>    boundary.  That means that future diffs are much more readable.
> 
>    http://rhodesmill.org/brandon/2012/one-sentence-per-line/
> 
> Ie, instead of this
> 
>>  Specifies the disks (both emulated disks and Xen virtual block
>>  devices) which are to be provided to the guest, and what objects on
>> -the host they should map to.  See L<xl-disk-configuration(5)>.
>> +the host they should map to.  See L<xl-disk-configuration(5)> for more
>> +details.
> 
> consider this:
> 
>>  Specifies the disks (both emulated disks and Xen virtual block
>>  devices) which are to be provided to the guest, and what objects on
>> -the host they should map to.  See L<xl-disk-configuration(5)>.
>> +the host they should map to.
>> +See L<xl-disk-configuration(5)> for more details.
>  
> You don't usually see the benefit of this approach in the patch where
> you start doing it, but it makes future edits clearer and easier.
> 
> I don't know whether you haven't been doing that in your changes
> because you didn't agree with it, or didn't want to do that as well as
> your other changes (in which case fine), or because you missed it or
> didn't understand it (hence the longer explanation here).
> 
> 
> Regards,
> Ian.
> 

Yes, I did understand your suggestion but I didn't act on it for two reasons.
First one was that I simply forgot about it once I actually got to editing the
man page to be honest, which was combined with me trying to have as few lines as
possible for some internal need to optimize on that front. I did indeed make
some linefeeds where I thought it would help to visually separate something. If
you don't think it's a problem I would rather leave that as is, because as you
can see, the patch is rather big and going through all that one more time would
be really time consuming. I will really try to keep that in mind for any future
edits.

kind regards,
Armando Vega

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel