[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] Is: Reviewed-by, Acked-by rules. Was:Re: [PATCH v7 10/19] x86/VPMU: Make vpmu not HVM-specific

>>> George Dunlap <dunlapg@xxxxxxxxx> 06/12/14 1:10 PM >>>
>I'm not sure why you *object* to non-maintainer Acks, Jan -- people
>who are either pushing the patch or applying the patch need to know
>who the maintainers are anyway, if for no other reason to know when a
>"Reviewed-by" counts as a maintainer Ack or not.
>It seems to me there are several times when non-maintainer Acks are
>useful; for example, when someone has previously objected to a patch,
>and is now OK with it, but has not actually reviewed it.

This is valid point. But other than unusual (I would think) situations like
this - what meaning would an ack of a non-maintainer really have? It's
not been a proper review (which, by refusing non-maintainer acks I would
hope we might get more of), and it also doesn't count for anything else
from a purely abstract pov. And I guess - back to the point you make -
there are other ways to express that reasons for previous objection
have got eliminated. After all - along the lines of what you said - the
person pushing a patch is expected to have followed an eventual
discussion relatively closely anyway.

>>>And I think George has a different opinion of when Reviewed-by
>>>get carried on patches that undergo modifications. Sadly I don't see
>>>anything in the Linux Documentation/Submitting* to explain what
>>>the rules are for that.
>> I don't recall having a "different" opinion here - at least I agree: With
>> non-benign changes to a patch, acks and reviews should be dropped.
>> Fixing bugs in a patch - which I think is the recent case you recall -
>> is clearly a non-benign change.
>> In fact I would go farther and suggest that more than a week or two
>> old tags should also get dropped. There have been a number of cases
>> not that long ago that new versions of patch series got posted weeks
>> if not months after the most recent earlier versions. And among them
>> have been cases where I wasn't really certain whether a tag with my
>> name was actually validly there, or valid anymore.
>FWIW, one example was Dario's soft-affinity series; when he was here
>before the hackathon we actually discussed what the protocol should be
>for that.  I didn't think it was clear, so I said he should retain
>them, and people could complain if that's not what they wanted. :-)

And iirc you then were the one who complained...

>For my part, I tend to appreciate being reminded of what my opinion
>was the last time around, so I don't think time itself should expire
>my tags.  The tags should expire when either the patch itself, or the
>code it depends on, has changed in a functionally significant way.  It
>should be the job of the person submitting the code to keep track of

Yes, I appreciate that there are reasons to retain old tags, and I didn't
mean to push (or initiate the pushing of) any new policy here. I merely
wanted to express that, as you say:

>The only risk to doing things this way, I suppose, would be if code
>the patch depends on had changed in a way that was incompatible, and
>the patch author didn't notice; the "Reviewed-by" might lull the
>maintainer into a false sense of security.

 ... there's a risk with retaining tags over long periods of time. Ideally of
course things would get followed up within shorter time distances, but I
know very well that this isn't always possible.


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.