qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Andreas Färber" <afaerber@suse.de>
To: Anthony Liguori <anthony@codemonkey.ws>
Cc: Peter Maydell <peter.maydell@linaro.org>,
	QEMU Developers <qemu-devel@nongnu.org>,
	Anthony Liguori <aliguori@amazon.com>,
	"Edgar E. Iglesias" <edgar.iglesias@gmail.com>
Subject: Re: [Qemu-devel] [PULL 0/6] target-arm queue
Date: Thu, 31 Oct 2013 17:52:31 +0100	[thread overview]
Message-ID: <52728ACF.9080403@suse.de> (raw)
In-Reply-To: <CA+aC4kukNzgcguvUNo=pOWYiWwup5AJBifFbo8MxUA8EO25MRQ@mail.gmail.com>

Am 31.10.2013 16:04, schrieb Anthony Liguori:
> On Thu, Oct 31, 2013 at 3:45 PM, Andreas Färber <afaerber@suse.de> wrote:
>> Am 31.10.2013 15:39, schrieb Anthony Liguori:
>>> On Thu, Oct 31, 2013 at 3:36 PM, Andreas Färber <afaerber@suse.de> wrote:
>>>> Am 31.10.2013 15:31, schrieb Peter Maydell:
>>>>> On 31 October 2013 14:18, Andreas Färber <afaerber@suse.de> 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	other threads:[~2013-10-31 16:52 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-25 18:07 [Qemu-devel] [PULL 0/6] target-arm queue Peter Maydell
2013-10-25 18:07 ` [Qemu-devel] [PULL 1/6] hw/arm/boot: Make user not specifying a kernel not an error Peter Maydell
2013-10-25 18:07 ` [Qemu-devel] [PULL 2/6] hw/arm: Tidy up conditional calls to arm_load_kernel Peter Maydell
2013-10-25 18:07 ` [Qemu-devel] [PULL 3/6] target-arm: Add CP15 VBAR support Peter Maydell
2013-10-25 18:07 ` [Qemu-devel] [PULL 4/6] target-arm: sort TCG cpreg list by KVM-style 64 bit ID number Peter Maydell
2013-10-25 18:07 ` [Qemu-devel] [PULL 5/6] target-arm: fix sorting issue of KVM cpreg list Peter Maydell
2013-10-25 18:07 ` [Qemu-devel] [PULL 6/6] integrator: fix Linux boot failure by emulating dbg region Peter Maydell
2013-10-31 14:02 ` [Qemu-devel] [PULL 0/6] target-arm queue Edgar E. Iglesias
2013-10-31 14:18   ` Andreas Färber
2013-10-31 14:21     ` Anthony Liguori
2013-10-31 14:31     ` Peter Maydell
2013-10-31 14:36       ` Andreas Färber
2013-10-31 14:39         ` Anthony Liguori
2013-10-31 14:45           ` Andreas Färber
2013-10-31 14:54             ` Anthony Liguori
2013-10-31 15:04             ` Anthony Liguori
2013-10-31 16:52               ` Andreas Färber [this message]
2013-10-31 16:54                 ` Anthony Liguori
2013-10-31 17:10                   ` Andreas Färber
2013-10-31 17:02                 ` Peter Maydell
2013-10-31 17:15                   ` Peter Maydell
2013-10-31 18:55                 ` Anthony Liguori
2013-10-31 15:16         ` Peter Maydell
2013-10-31 17:14           ` Andreas Färber
2013-10-31 17:18             ` Peter Maydell
2013-10-31 17:27               ` Andreas Färber
2013-10-31 17:51                 ` Peter Maydell
2013-10-31 18:58             ` Anthony Liguori
2013-10-31 22:13     ` Edgar E. Iglesias
  -- strict thread matches above, loose matches on Subject: below --
2014-03-19 12:05 Peter Maydell
2014-03-19 13:33 ` Peter Maydell
2016-07-07 13:48 Peter Maydell
2016-07-11 10:16 ` Peter Maydell
2018-07-30 14:17 Peter Maydell
2018-07-30 18:11 ` Peter Maydell
2018-10-29 15:34 Peter Maydell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=52728ACF.9080403@suse.de \
    --to=afaerber@suse.de \
    --cc=aliguori@amazon.com \
    --cc=anthony@codemonkey.ws \
    --cc=edgar.iglesias@gmail.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).