Re: [Xen-devel] [PATCH for-4.13] xen: Drop bogus BOOLEAN definitions, TRUE and FALSE

[Jan Beulich <jbeulich@xxxxxxxx>]

On 06.01.2020 20:01, Andrew Cooper wrote:
> On 09/12/2019 12:11, Jan Beulich wrote:
>> On 06.12.2019 22:02, Andrew Cooper wrote:
>>> On 12/11/2019 14:03, Jan Beulich wrote:
>>>> Bottom line - I'm half convinced and willing to give my ack, but
>>>> I'm not convinced you truly thought through the longer term
>>>> consequences. I'd therefore be far happier to see this patch
>>>> split into a non-controversial part (anything that's not tied to
>>>> the ACPI and EFI header imports), an ACPI, and an EFI part.
>>> I do not want to writing the same patch again in $N years time because
>>> review and CI missed it creeping back in.
>>>
>>> I don't think this is an unreasonable position to take.
>> It for sure isn't. Yet I also don't think though my request how to
>> split things is. By asking for the split I'm implying that we may
>> still reach agreement on the controversial parts, faod. Sadly once
>> again there are no other opinions helping to sort which route may
>> be the overall preferred one.
> 
> As Julien points out (and you not responded to his email) you have at no
> point actually requested a split.  You merely suggested that you might
> be willing to ack 1/3 the patch, with the overt implication that
> objections would continue on the rest.

Please read again the second sentence of my earlier reply still
visible in context above. (As an aside, I've not seen a message
from Julien to the effect of me not having requested a split. All
I have in my inbox is him pointing out that I didn't explicitly
ask for other people's opinions.)

> This deadlock has gone on far too long.  Therefore, you have until the
> end of the week to either:

I'm sorry, but very much NO to the underlying principle. It is so
much more frequently that my patches are stuck on you dropping a
discussion in the middle (or not even replying to patches in the
first place) that I don't think you're in a legitimate position
to set this kind of an ultimatum (leaving aside that from my
perspective there's no deadlock here at all, as per the part of
my original reply that you've left in context above).

> 1) Agree in principle that you will accept this patch (modulo tidy-up)
> split into 3 - i.e. that I won't be wasting my time doing so, or

Please read again the first sentence of my earlier reply still
visible in context above. The split request was really because
the changes to the cloned sources / headers want a different
justification (for example, "buggy" is not a valid attribution
for BOOLEAN to be a typedef of unsigned char - what can be buggy
as a result are wrong uses of variables of this type, just like
was the case for our original bool_t - and it's instead a very
legitimate means to arrange for code to be re-usable across a
wide range of used C compilers) than the ones to sources that
we don't consider "cloned" (anymore). In particular, as
expressed, I'd like the descriptions to make at least implicitly
clear that the longer term consequences have been considered: A
dozen small changes like this one are as impactful to later
re-sync attempts as is one big change. However much I'm in favor
of not taking pure black-and-white perspectives, I'm afraid
there's not much of an alternative here.

> 2) Provide a concrete alternative which retains the property of being
> impossible for buggy constructs to find their way back into the codebase.

I stand by my original statement: If you don't want to waste time,
submit just the uncontroversial third. The concrete alternative is
for people (submitters even more than reviewers) to be disciplined:
Don't use what you call "buggy" constructs in unsuitable contexts.

I'd be willing to withdraw my (half) objection (seeing that you've
got Stefano's ack and Wei's R-b) if we could settle the underlying
discussion: If I'm not mistaken, in some cases you (and others)
have asked for clones of foreign repo sources to stay as close to
the original as possible (sorry, don't have an example to hand),
yet here you're going the opposite direction. Quoting from an
earlier reply of yours: "We do not need to take the headers
verbatim, and there are good reasons to specifically not take them
verbatim." I agree, but there should be clear guidelines (if not
rules). I.e. the decision which way to go for any individual piece
of code should ideally be independent of the person doing the
actual import or change (and - long term, as correcting past
mistakes does take time - be consistent across the code base).

Perhaps this would best be made a topic for the community call
(iirc scheduled for later this week). I'll try to remember to put
it up as a topic.

I'm sorry, it's clear this isn't exactly the reply you've been
wanting.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel