lack of reviewers

[Peter Hutterer]

On 18/05/12 21:17 , Michal Suchanek wrote:
> On 18 May 2012 12:40, Peter Hutterer<peter.hutterer at who-t.net>  wrote:
>> On 18/05/12 19:26 , Michal Suchanek wrote:
>>>
>>> On 18 May 2012 01:14, Peter Hutterer<peter.hutterer at who-t.net>    wrote:
>>>>
>>>> On Thu, May 17, 2012 at 10:39:55AM +0200, Ernst Sjöstrand wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> (sorry for jumping in from the outside and breaking the thread!)
>>>>>
>>>>> I read about this problem and wanted to offer a suggestion!
>>>>>
>>>>> What if you set up a Gerrit server for git.freedesktop.org? That's the
>>>>> tool the Android OpenSource project uses among other things:
>>>>> https://android-review.googlesource.com/
>>>>> Perhaps if it was easier to contribute to reviewing code, more people
>>>>> would do it more often?
>>>>
>>>>
>>>>> It's also a very nice tool I have to say, I use it every day at work.
>>>>> It's easy to integrate with automatic
>>>>> testing of patchsets before they're submitted to the repository for
>>>>> example.
>>>>
>>>>
>>>> tbh I doubt what we have is a tool problem. Patches are sent to the list
>>>> and
>>>> can be reviewed quite easily there (for subscribers, anyway). The issue
>>>> we
>>>> have is manpower and, more importantly, manpower of people with enough
>>>> knowledge to judge whether a patchset has side-effects beyond the
>>>> obvious.
>>>>
>>>> in the end, such patches tend fall on the shoulders of a few and adding
>>>> another tool that they have to check will increase, not decrease, the
>>>> workload for those.
>>>
>>>
>>> tbh using a mailing list for that looks very impractical.
>>>
>>> - patches get missed completely
>>
>>
>> true. we do encourage people to re-submit. which, aside from the obvious
>> disadvantage, has advantages too. I found the problem with any todo list is
>> that sooner or later it becomes so long that you either have to wipe it (by
>> switching to a different system sometimes) or you start ignoring stuff
>> anyway.
>>
>> given that one of the problems is reviewer bandwidth, I expect this to
>> happen with any new tool. patchwork was great in the first few weeks, now
>> it's a kitchensink great for getting URLs and not much else.
>
> Given that reviewer bandwidth is scarce it would be perhaps helpful to
> spare it by having a tool that presents all the not-yet-reviewed
> patches in one place rather than reviewers fishing for them in the
> mailing list or in the patchwork.

I believe that patchwork was supposed to be that tool, but either it's 
not set up correctly or it just doesn't do that. I don't know but if 
patchwork can do this, then this would be a simple immediate step.

having said that - it can't be perfect without manual intervention - 
when patches go through multiple revisions someone still has to mark 
them manually as obsolete.

>> requiring people to ping when patches get missed at least notifies us which
>> patches have people behind them that care. a feature that gets submitted
>> once, forgotten and no-one pings for it may not have been that important to
>> begin with.
>
> When you get the 5th patch for the same regression submitted the third
> time it starts to look like shouting your patches off a cliff in the
> dead of the night.

Usually it helps directly CC-ing the maintainer of that subsystem when 
the patch is ignored the first time. note - this only works because we 
don't get CCd on every patch, so the signal/noise ratio is still 
reasonably good. (Also, I say "we", the above is true for me so I just 
assume it's similar for others :)

At several times in the past we noted the need for someone to just 
collect leftover patches but that never happened, usually due to lack of
time. we really need someone to actually do it.

>>> - there is no track of what is related to what (as in the part of the
>>> same patchset or new revision of the same patchset)
>>
>>
>> patch are usually in numbered series, in threads, with new revisinos being
>> prefixed with "v2", "v3", etc. requires submitter discipline but it works to
>> some degree.
>
> And as some of the patches get replies they get out of order and
> completely out of context.
>
>>
>>
>>> - you get a lots of list noise due to patches being sent one by one
>>
>>
>> I'm not sure I follow this argument, can you expand?
>
> Like a series of 10+ patches for some part of the X server I do not
> understand landing on the list several times.
>
> I guess some people are fond of replying to the patches and quoting
> them in their email client and I can see that as nice feature but it's
> paid for by tons of list traffic. Necessarily large part of that is
> meaningless to most.
>
> The alternative is, of course, a link to git branch or something like that.

git branches are _incredibly_ painful to review. they're good for testing, 
but the steps involved to point out an issue in a specific hunk of a
specific commit involves a lot more steps than just hitting reply and
starting to type.

Cheers,
  Peter