qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PULL 0/6] target-arm queue


From: Andreas Färber
Subject: Re: [Qemu-devel] [PULL 0/6] target-arm queue
Date: Thu, 31 Oct 2013 17:52:31 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.0

Am 31.10.2013 16:04, schrieb Anthony Liguori:
> On Thu, Oct 31, 2013 at 3:45 PM, Andreas Färber <address@hidden> wrote:
>> Am 31.10.2013 15:39, schrieb Anthony Liguori:
>>> On Thu, Oct 31, 2013 at 3:36 PM, Andreas Färber <address@hidden> wrote:
>>>> Am 31.10.2013 15:31, schrieb Peter Maydell:
>>>>> On 31 October 2013 14:18, Andreas Färber <address@hidden> wrote:
>>>>>> Peter, since I had picked up the first two patches into my still pending
>>>>>> qom-next pull, as per the QEMU Summit discussion those patches should've
>>>>>> gotten an Acked-by.
>>>>>
>>>>> Hmm? I don't recall this part of the discussion. If you want the
>>>>> patches to have an Acked-by from you you need to send mail
>>>>> to the list with an Acked-by line.
>>>>
>>>> No, I added a Signed-off-by. It was clearly stated that a Reviewed-by
>>>> needs to be explicitly sent as reply but that "looks okay" should in
>>>> exactly such a case where sender=submaintainer should be recorded as
>>>> Acked-by, and Sob is certainly stronger than Acked-by. Cf. minutes.
>>>
>>> Nope.  If you want there to be an Acked-by, say "Acked-by:".  Don't
>>> make people infer your Acked-bys.
>>
>> Yes, that's in the minutes. And yes, that's what I got as answer there.
>> Please reply to the minutes if you think otherwise.
> 
> I explicitly said that Acked-bys are useless too.
> 
> The minutes say that you said the kernel treats "Acked-bys" as "looks
> good".  You did say that.

I *asked* about what to do with my QEMU CPU patches that only get a
"looks okay" and got only positive answers for whether that should be an
Acked-by and no objection, including none from you.
I certainly said nothing at all about the kernel.

>  At no point did a "rule" get made though.

The new rule you made was: no patch without Reviewed-by.
Peter sending that PULL and Edgar merging it both violate that rule.
No objection against a particular patch function-wise.

Point is, had Peter ping'ed me before sending out that pull, he would've
actually gotten a Reviewed-by from me, thereby satisfying your rule! He
didn't, ignoring that he himself had actually told me to queue the
patches before his vacation, for which obviously I reviewed and tested them.

Maybe there's no obligation for picking up tags, but then again you
can't go ahead and do statistics over incompletely recorded tags.

Regards,
Andreas

>> I brought up exactly this situation where I am contributor to CPU and
>> submaintainer of CPU and often not getting Reviewed-bys but if at all,
>> such as from Paolo recently, some verbal "looks OK" for a series. I was
>> told that that should be turned into an Acked-by on the patches to
>> satisfy your criteria that contributors may not just send patches as
>> pull without Reviewed-by.
> 
> I think you misunderstood.
> 
> I don't care about Acked-bys.  They are useless.
> 
> A third of patches are being committed with Reviewed-bys.  There are
> certainly many cases where patches are going in from submaintainers
> that have been reviewed which comes implicitly with Signed-off-by.
> 
> But I worry that we're not reviewing enough on list and that there are
> patches from maintainers going in through maintainer trees that aren't
> getting outside review.
> 
> There's no immediate action for this other than we should all try to
> review more patches on list to prevent the above situation.
> 
>>> And adding tags is a nice-to-have.  There is no "rule" stating that
>>> you must include everyone that appears on the mailing list.  But I
>>> expect that maintainers try to
>>
>> Again, at QEMU Summit you pushed for making Reviewed-by a must-have and
>> we discussed whether a submaintainer must add a Reviewed-by then and
>> what to do if author==submaintainer. If you dropped that thought, then
>> fine with me.
> 
> Yes, patches should get reviewed.  I hope this is obvious to all of us :-)
> 
> I also suggested that I have tooling that people can use to simplify
> adding collected Reviewed-bys on the list.
> 
> But none of this has anything to do with inferred Acked-bys.  I'll go
> a step further and say that I would be very unhappy if anyone every
> added any kind of tag to a patch with my name on it that I didn't send
> myself.
> 
> Regards,
> 
> Anthony Liguori
> 
>>
>> Regards,
>> Andreas
>>
>> --
>> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
>> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



reply via email to

[Prev in Thread] Current Thread [Next in Thread]