[OpenAFS-devel] Gerrit reviews and the rate of acceptance

[Matt W. Benjamin]

Hi Jeff,

I don't think number of reviewers is the limiting factor.  For example, I submitted a change to enhance Simon's contributed red-black tree.  Simon himself reviewed it, had no objections to committing the change.  I don't care if the change is committed, but I don't think you can legitimately fault non-present reviewers.

That's not to say we don't need more reviewers, we surely do.  If you'd like me to review work more frequently, a good way would be to ask me to do so.  Gerrit has a feature to "add reviewer" which can be used for this purpose.

More broadly, I have written a lot of code for OpenAFS over the years, much of it not committed.
So have others (e.g., Hartmut).  There are simply too many reasons to list why those contributions cannot be committed.  Most saliently, I've been told not to send more XCB changesets, dirformat changesets, other random YFS-funded changes, for many good reasons.  They are all to do with larger change management policy, such as that governing afs3-stds.

Regards,

Matt

----- "Jeffrey Altman" <jaltman@your-file-system.com> wrote:

> This week I have received private mail from several individuals which
> are concerned about the rate of OpenAFS development.  There were
> several
> topics raised across the e-mails and I'm going to pick out themes and
> try to address them in separate mails to this list and openafs-info
> as
> appropriate.
> 
> The first theme is that it takes too long for patches to be accepted
> into OpenAFS.  As evidence of the overwhelming delays is the nearly
> 100
> patchsets that are sitting in Gerrit for more than two weeks.  Many
> of
> which have been sitting there for nearly two years.
> 
> Many of these patchsets are ideas that were sent to Gerrit either so
> they wouldn't be lost or so others could use them as a starting
> point.
> Some of the patchsets are items that have been reviewed but which
> have
> outstanding issues which were never addressed.  But the vast majority
> of
> the items sitting in Gerrit for an extended period of time are
> patches
> that have simply not received sufficient reviews to decide what to do
> with them.
> 
> The fact is that the number of active reviewers is exceedingly small.
> Here are the statistics for the 4180 patches contributed to the
> 'master'
> branch since Gerrit was deployed.
> 
>    3178     Reviewed-by: Derrick Brashear
>    1355     Reviewed-by: Jeffrey Altman
>     217     Reviewed-by: Simon Wilkinson
>     155     Reviewed-by: Russ Allbery
>     110     Reviewed-by: Andrew Deason
>      68     Reviewed-by: Marc Dionne
>      51     Reviewed-by: Rod Widdowson
>      49     Reviewed-by: Asanka Herath
>      43     Reviewed-by: Alistair Ferguson
>      24     Reviewed-by: Peter Scott
>      21     Reviewed-by: Matt Benjamin
>      15     Reviewed-by: Tom Keiser
>      12     Reviewed-by: Dan Hyde
>      11     Reviewed-by: Michael Meffie
>       9     Reviewed-by: Chaz Chandler
>       6     Reviewed-by: Hartmut Reuter
>       6     Reviewed-by: Benjamin Kaduk
>       5     Reviewed-by: Stefan Kueng
>       5     Reviewed-by: Garrett Wollman
>       3     Reviewed-by: Jacob Thebault-Spieker
>       2     Reviewed-by: Thomas L. Kula
>       2     Reviewed-by: Steve Simmons
>       2     Reviewed-by: Phillip Moore
>       2     Reviewed-by: Mickey Lane
>       2     Reviewed-by: Jonathan A. Kollasch
>       2     Reviewed-by: Jeffrey Hutzelman
>       2     Reviewed-by: Claudio Bisegni
>       2     Reviewed-by: Adam Megacz
>       2     Reviewed-by: Ken Dreyer
>       1     Reviewed-by: sanket
>       1     Reviewed-by: Tharidu Fernando
>       1     Reviewed-by: Steven Jenkins
>       1     Reviewed-by: Stephan Wiesand
>       1     Reviewed-by: Rainer Toebbicke
>       1     Reviewed-by: Jason Edgecombe
>       1     Reviewed-by: Christof Hanke
>       1     Reviewed-by: Chaskiel Grundman
>       1     Reviewed-by: Chas Williams
>       1     Reviewed-by: Alexander Ivan Redinger
>       1     Reviewed-by: Alex Chernyakhovsky
> 
> All reviews are performed by volunteers.   It is nobody's day job to
> perform Gerrit reviews.  Obviously Derrick, Russ and I as the
> gatekeepers that review patches as part of the approval process have
> performed the largest number of reviews.  A special note of thanks to
> Simon, Andrew, Marc and Rod for being at the top of the
> non-gatekeepers
> list.
> 
> In the last year alone there have been 44 contributors whose work has
> been committed to openafs master.  Yet there are fewer than that
> number
> of individuals who have reviewed someone else's work since Gerrit was
> implemented.
> 
> No one is being paid to review code.  Not even the gatekeepers.  The
> nearly 100 patchsets that are sitting in Gerrit represent
> approximately
> 2% of all of the patches that have been submitted.  The role of the
> gatekeepers in the review process is important but they should not be
> the sole review mechanism for OpenAFS submissions.
> 
> OpenAFS needs a much more active reviewer community.
> 
> If we were to impose a requirement that every patchset must receive a
> total vote of +4 or +5 before it could be submitted, would that help
> to
> obtain reviews?
> 
> The floor is open for discussion.
> 
> Jeffrey Altman

-- 
Matt Benjamin
The Linux Box
206 South Fifth Ave. Suite 150
Ann Arbor, MI  48104

http://linuxbox.com

tel. 734-761-4689
fax. 734-769-8938
cel. 734-216-5309