Re: [Xen-devel] [v7][PATCH 00/16] Fix RMRR

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 16.07.15 at 11:27, <George.Dunlap@xxxxxxxxxxxxx> wrote:
> On Thu, Jul 16, 2015 at 9:26 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>>> On 16.07.15 at 10:13, <tiejun.chen@xxxxxxxxx> wrote:
>>>>>>> It looks like most of the libxl/libxc patches have been acked.  It
>>>>>>> seems to me that most of the hypervisor patches (1-3, 14-15) are
>>>>>>> either ready to go in or pretty close.
>>>>>>
>>>>>> Now that I looked over v8 I have to admit that if I was a tools
>>>>>> maintainer I wouldn't want to see some of the tools patches in
>>>>>> with just an ack, but without any review.
>>>>>
>>>>> I'm somewhat confused at this point.
>>>>>
>>>>> Acked-by: is often used by the maintainer of the affected code when that
>>>>> maintainer neither contributed to nor forwarded the patch. It is a
>>>>> record that the acker has at least reviewed the patch and has indicated
>>>>> acceptance.
>>>>>
>>>>> Does this imply this is already reviewed?
>>>>
>>>> No, that would be expressed by Reviewed-by. Acked-by merely
>>>> means no objection by the maintainer for the change to go in.
>>>>
>>>
>>> Sorry I'm trying to dig into this.
>>>
>>> If nobody would like to take a look at this, so isn't this the
>>> associated maintainer's responsibility to review finally? In this case
>>> isn't Acked-by fine enough?
>>
>> Acked-by is good enough for a patch to go in, yes. Note that I
>> didn't make this a requirement (as I'm not the maintainer), I just
>> said that if I was the maintainer, I would for at least some of the
>> tools patches.
> 
> There does seem to be a disconnect between how "Reviewed-by" and
> "Acked-by" are used on the tools side vs the hypervisor side.  (We
> just stumbled across this in an internal discussion about commit stats
> actually.)
> 
> But in any case, it's the maintainers' responsibility to determine if
> something has had sufficient review, and it's their responsibility not
> to give an Ack unless they really mean "As far as I'm concerned, this
> is ready to go in."  The fact that there were Acks on the toolstack
> side ought to mean that this judgement had already been made.

Hmm, that's a different view than I take: To me Reviewed-by implies
Acked-by, but not the other way around. And I view it as the
committer's responsibility to ensure a patch has all necessary acks,
but not the maintainer to give an ack only when reviews were done.

In the end I'm afraid the current model of tags may not be suitable
to express everything we need, or the lack of a formal description
somewhere leaves too much room for mismatching interpretations.

Jan


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