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

[Simon Wilkinson]

On 11 Apr 2012, at 18:09, Matt W. Benjamin wrote:
> 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.

The rbtree change actually exposes some additional issues, so whilst it =
is a single example, I think it is worth exploring further.

Whilst the rbtree change is simple, in and of itself, it introduces a =
CUnit based set of tests. These tests aren't useful without a CUnit test =
driver, as pointed out during the review process. In fact, we probably =
don't want to push the rbtree change without that driver, given the =
general problem that we have with dead, unbuilt, code in the tree.

6242 is that CUnit test suite driver. However, this raises two broader =
questions. A number of developers have spent significant effort on =
building TAP based tests, building on the work that Russ did originally =
to hook his TAP test suite up into the OpenAFS build machinery. Do we =
really want to support two different test suites, and two different test =
suite drivers in OpenAFS? In addition, some of the code in 6242 is CDDL =
licensed, which adds yet another copyright to the OpenAFS licensing =
swamp, and may well cause problems for distributions that are CDDL =
averse.

Those issues with 6242 are what is blocking the (trivial) rbtree change. =
In both cases these issues have been commented on in review, but no =
resolutions agreed upon. If anything, the principal problem with our =
review process is that unless the original submitters drive their =
patches forwards, by proactively responding to review comments, and =
uploaded amended patchsets, then things will stall.

Cheers,

Simon.