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. _______________________________________________ OpenAFS-devel mailing list [email protected] https://lists.openafs.org/mailman/listinfo/openafs-devel
