Re: [Qemu-devel] An organizational suggestion

[Anthony Liguori <anthony@codemonkey.ws>]

Ian Jackson wrote:
> Paul Brook writes ("Re: [Qemu-devel] An organizational suggestion"):
>   
>> I'd just like to point out that committing patches is easy. The hard
>> (and time consuming) bit is identifying, rejecting, fixing and/or
>> making constructive comments on all the bogus patches. The are lots
>> of patches that fall into this latter category, e.g. patches that
>> have clearly only ever been tested on x86 targets.
>>     
>
> Yes.
>
>   
>>  It doesn't take any special privileges to do this patch review.
>>     
>
> That's true, but it doesn't tell the whole story.  Anyone can
> criticise a patch (and we do).  But while review by a non-committer is
> very helpful, it still doesn't mean that the committer doesn't have to
> do review of their own.  After all the committer is actually the
> gatekeeper and has the personal responsibility to commit good code.
>
> Also, review and improvement of patches by non-committer contributors
> depends on the contributors' expectation that patch will be accepted
> when it is good.  There is no point in people reviewing and commenting
> on and improving patches if the maintainers don't have the time or
> inclination to get those refined patches actually reviewed by them and
> committed.
>   

Here's a couple things I notice that often cause a patch to be dropped:

1) It has something wrong, but not sufficiently interesting enough for 
anyone to offer feedback

2) It has something wrong, there's a thread with discussion and 
resubmissions of the patch, and the thread eventually cools off with a 
patch people are happy with

3) It's lost in the noise

#3 is something that happens.  A contributer has to resend patches to 
any project.  I don't think this happens very often on QEMU.

More often, I think the problem is #1.  This is something that we all 
can fix by just reviewing patches.  This has been an important issue for 
a while now and I'm willing to dedicate a portion of my time to 
reviewing patches.  For anyone submitting a patch to qemu-devel, feel 
free to CC me to make sure I review it.  I'm not saying that my review 
will guarantee acceptance, but I will keep track of patches I've reviewed.

#2 is a process problem.  It's unclear in this sort of thread, who's 
happy with the patch, and whether a conclusion has been reached.  The 
best thing to do is here to make use of Reviewed-by or Acked-by tags and 
to resubmit the patch as a top-level posting once consensus has been 
reached.

With a few more people dedicating time to patch review I think we can 
improve things quite a bit.

Regards,

Anthony Liguori