Re: [Xen-devel] differing opinions between maintainers vs patch acks

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 04.05.17 at 14:44, <ian.jackson@xxxxxxxxxxxxx> wrote:
> Andrew Cooper writes ("Re: differing opinions between maintainers vs patch 
> acks"):
>> Taking this example, as you have called it out, but without going into
>> the details.
>> 
>> I accept that the issues under debate do not have any impact on the
>> technical correctness of the fix.  Once compiled/assembled, the binary
>> will function correctly.
>> 
>> However, the bikeshedding makes a very real material impact on the
>> understandability and reviewability of the code.
>> 
>> In my mind, all other things being equal, making the code easier to
>> understand and review is of paramount importance, and particularly in
>> this case, not making an already complicated bit of code harder to review.
> 
> Well, at one level I agree with Andrew on at least the 1*1 and 0*8
> question.  These seem clearer to me as they state the programmer's
> intent as well as merely the effect.  I found Jan's response hard to
> understand; there doesn't actually seem to be a counterargument.

My counterargument was that 0*8 clearly equals 0 for anyone
knowledgeable enough to read this code, as does 1*8 = 8.
Anyway, seeing that you agree with Andrew, I'll go make the
change, no matter that I think it doesn't belong here (besides
being pointless).

>  I
> suspect if I thought about it enough I would agree with Andrew about
> the labels too.

Along those lines I'd then also go make the change here, if only
there was an alternative naming of the label tags that I can at
least live with; the suggestion to simply divide the numbers by 8
is, as expressed, not something I consider reasonable. So I'll
make my changing of those label tags dependent one someone
coming forward with a naming scheme which is both better than
what is there and better then using simple stack slot numbers
without it being clear that stack slots are being meant.

> But, earlier I said:
> 
>    I definitely agree that there is room for giving the author of some
>    code (whether they are a maintainer or not) some leeway on matters of
>    taste.  I think, though, that while this ought to be a principle
>    applied by maintainers, committers and anyone else making relevant
>    decisions, it is not a rule of governance to be applied in contested
>    cases.
> 
> I think this case falls clearly into the category of things where we
> could give the original contributor some leeway.  In this case that
> means Jan.
> 
> IOW if I were in Andrew's position I would probably make the same
> requests he has done, but if Jan maintained his position I would
> certainly not block the patch over this.
> 
> 
> Stepping back a bit: It is indeed important that our code is easy to
> understand and modify, expresses its intent clearly, and helps future
> programmers avoid writing bugs.  But it is also important that
> contributors feel valued, and feel a sense of ownership.
> 
> The amount of emotional discouragement to a contributor does not scale
> linearly with the size and apparent importance of the disagreement.
> Indeed, turning a tiny issue into a blocker or a big argument can be
> especially demotivating.
> 
> I think this case is an example of a situation where we should pay a
> small price in code readability to keep a contributor happy.  (That
> the contributor is also a maintainer doesn't seem to change this
> analysis for me.)
> 
> 
> I doubt either side will be particularly happy with this analysis.
> Sorry about that.

No reason to be sorry - happy or not, your reply at least gives me
an understanding of how others think here.

Jan


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