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: Anthony Liguori
Subject: Re: [Qemu-devel] [PULL 0/6] target-arm queue
Date: Thu, 31 Oct 2013 17:54:15 +0100

On Thu, Oct 31, 2013 at 5:52 PM, Andreas Färber <address@hidden> wrote:
> 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.

I never said anything like that.

Regards,

Anthony Liguori

> 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]