Re: [Qemu-devel] We need more reviewers/maintainers!!

[Alexander Graf]

Am 13.03.2012 um 01:16 schrieb Anthony Liguori <address@hidden>:

> On 03/12/2012 06:32 PM, Andreas Färber wrote:
>> Am 12.03.2012 22:18, schrieb Anthony Liguori:
>>> On 03/12/2012 04:12 PM, Stefan Weil wrote:
>>>> Am 12.03.2012 21:27, schrieb Anthony Liguori:
>>>>> On 03/12/2012 03:12 PM, Stefan Weil wrote:
>>>>>> [...] There a many examples of
>>>>>> urgent patches (= patches which fix broken builds) which take
>>>>>> several days even when they were reviewed before they finally
>>>>>> are committed.
>>>>> 
>>>>> Can you be specific? [...]
>>>> 
>>>> I don't mean w32 patches only. The last build break was for ppc hosts
>>>> (e04b28996110bd6acfc059e9f2c8c5aba5119a46, it took more than 5 days),
>>>> but I remember also situations were x86 was broken more than
>>>> a day.
>>>> 
>>>> A selection of older patches which fixed build and the time it took
>>>> from creation to commit:
>>>> 
>>>> 6148b23d69444a300710db0c53f6c53b7f3c8067 (kvm ppcm 3 days)
>>>> 1ecf47bf0a091700e45f1b7d1f5ad85abc0acd22 (w32, 2 days)
>>> 
>>> So both of these platforms have maintainers that can do PULL requests
>>> for issues like this.
>> [snip]
>> 
>> You're missing the point here: There usually *are* patches quickly, but
>> they often don't get applied to qemu.git as fast. Often there are even
>> two or three people sending build fixes concurrently. Do you really need
>> a PULL for a single patch that affects everyone?
> 
> A PULL is just as good as a push.  There's a slight lag in a PULL, but 
> generally it should be pretty quick.
> 
>> 
>> Take Blue's recent target-ppc fix
>> 9d4df9c02866f39d3eef105033091f367cc7c29e for example: After applying
>> patches on day one of FOSDEM he posted a -Werror fix, it got confirmed
>> by me and Alex but wasn't applied until a week later, because apparently
>> no other committer dared to apply Blue's patch despite SoB and acks and
>> people reporting the issue... Not happy.
>> No doubt Alex is to blame for not catching that issue in his ppc queue,
>> but asking Alex as submaintainer to submit a PULL for a single patch
>> posted by Blue as committer seems overly complicated to me! ;)
> 
> I think this is a good demonstration of what the problem is.  Unclear 
> responsibility.  I'm pretty sure that Blue thought that Alex would handle the 
> patch.  I'm pretty sure that Alex thought Blue would handle the patch.
> 
> If we start saying that, Alex "owns" ppc except for things that are 
> "important" like a build breakage, then we get into the ugly definition of 
> what's important and what's not important.
> 
> So yes, if the build breakage is limited to PPC, and came in through the PPC 
> tree, I'd expect the fix to also come through the PPC tree.

Yeah, the ppc tree issue is probably special to my workflow. I tend to only 
keep a single branch - ppc-next. That branch is basically my staging and 
development tree which I synchronize using pull requests every now and then.

The problem with that is that single build breakage fixes don't fit in at all 
in that workflow - and I haven't found a good alternative yet. Jumping through 
different trees just increases chances I lose track of patches even more, so 
splitting ppc-next and ppc-something-i-commit-now doesn't work for me.

And yes, pull requests do take longer. They are also more visible than pushes. 
But that also means that the barrier for doing one is bigger - who really wants 
to spam the ML? Sending a pull request is also a matter of minutes, while a 
push is a matter of seconds. Smoke tsting of that tree alone is a matter of 
running autotest for half a day. It sounds like no big deal, but I often found 
myself not pushing because I felt the I don't have enough patches queued up to 
make a pull request worthwhile, while I do push wholeheartedly to ppc-next, 
since I assume that tree unstable.

This is all founded in the whole idea behind doing pull requests though, they 
are supposed to be visible. They are supposed to be scarce. They are supposed 
to be consistent, well tested states. This is orthogonal to the requirements of 
quick fixes, which are usually very simple and need to be applied as fast as 
possible and don't need deep testing because they are very limited in scope.

Either way, next time I'll just mail Blue directly that he should apply a 
single commit fix and then everyone'll be happy :).


Alex