Re: [Ksummit-discuss] [TOPIC] Encouraging more reviewers

[Dan Williams <dan.j.williams@intel.com>]

On Sat, May 24, 2014 at 2:53 AM, James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
> A lot of the problems accepting patches into the kernel stem from trying
> to get them reviewed ... some patch series even go for months without
> being reviewed.  This is caused by a couple of problems.  The first one
> is a submission issue, where you get a long N patch series (usually N >
> 10) you review and provide feedback, then you get a long N+Y series back
> eventually with no indication what disposition was taken on any of the
> review points ... even the most hardy reviewers get a bit demotivated at
> this point. The second is an actual lack of reviewers in particular
> fields, meaning that a small number of people tend to be the ones
> reviewing patches in particular subsystems.
>
> The former is probably just an addition to SubmittingPatches explaining
> how to resend after addressing review comments.  The latter was supposed
> to be helped by having the Reviewed-by: tag so we gave credit to
> reviewers.  I've found the Reviewed-by tag to be a bit of a double edged
> sword: it is a good way of giving review credits, but I also see patches
> that come in initially with it on (usually the signoff and the
> reviewed-by are from people in the same company) ... it's not
> necessarily a bad thing, but it doesn't add much value to the kernel
> review process, because we're looking for independent reviews.  The
> other thing I find problematic is that some people respond to a patch
> with a Reviewed-by: tag and nothing more.  I'm really looking for
> evidence of actually having read (and understood) the patch, so the best
> review usually comes with a sequence of comments, questions and minor
> nits and a reviewed-by at the end.
>
> However, the fundamental problem is we still need to increase our review
> pool, so we need more than the Reviewed-by: tag.  It has been suggested
> that people who submit patches should also be required to review patches
> from other people ... I think that's a bit drastic, but it is a
> possibility. Another possibility for the kernel summit might be to have
> review time instead of hack time: we each nominate a patch set needing
> review and it gets reviewed by others in that time ... if that one's
> successful, we could extend the concept to other conferences and
> development sprints which do hack time.  Another possibility is to
> publish a subsystem to review list (similar to the to do list idea).
> This at least shows submitters why their patch series is languishing and
> could serve as a call to arms if it gets too long.

Mandatory patchwork?  If no one can perceive the patch queue depth how
do they know where to help?  Would also be nice if patchwork had
states like "needs review from another fibre channel developer" to
make it explicit the class of review that is being sought from the
maintainer.

> I'm sure there are many other things people could suggest.

Forgive the co-opt / tie-in to my own topic proposal, but I think one
way to increase review is to do it after the code is merged.  Quality
reviews tend to also show up after bug reports.  So, maybe finding a
way to get more code in people's hands, in a responsible way, also
increases the quantity and quality of reviews.

--
Dan