On 4/11/2012 2:23 PM, Jason Edgecombe wrote: > Is there an agreed-upon criteria for review? If so, where are the > criteria? I ask this as I consider my "C" skill to be poorly developed.
There is no list of criteria for reviewers. However, the value of the review is certainly bounded by the quality and experience of the reviewer. > What is the goal for patchset review? More eyeballs on fewer patches, > fewer eyeballs with more patches? In other words, where do we want to be > on the spectrum or quality vs. quantity? The rule for patchset submission is that its size should be small enough that a skilled reviewer with strong familiarity of the code should be able to perform a thorough review in under an hour. Patchsets should not contain more than one unrelated change. We are doing a pretty good job with the size of the patchsets being submitted. Simon and Marc probably do the best job when it comes to breaking up large changes into a set of manageable patchsets. One thing that is certainly true. The larger the patchset the fewer people are going to review it and the longer it will sit in Gerrit. > Other ideas: > * giving more people rights to approve a patchset (more gatekeepers?) The rate of approval is not the problem. The title "gatekeeper" is something that is earned. The core tasks that are performed by a gatekeeper can be performed by any member of the community. If there is a patch to be written, any one can write it. If there is a bug to be fixed in RT, any one can read the bug and submit a patch to gerrit for it. If there is a patch to be reviewed, any one can review it. If there is testing to be done (and there always is), any one can test. Gatekeepers time is extremely valuable because it is limited. The more patch writing, bug fixing and reviewing that can be done by others, the easier it is for the gatekeepers to "approve and submit" patches and focus on bigger architectural issues. > * automatically approving a patchset after the voting threshold is met > (robo-gatekeeper?) I would be strongly opposed to this. Gatekeepers must have the ability to veto a patchset due to any number of issues. > * different levels of votes with an increased voting threshold. (i.e. > gatekeepers =3 votes, active contributors=2 votes, everyone else=1 vote. > 3 votes approves a patch?) I'm not sure that having a voting threshold is a good idea. However, I selected +4 and +5 intentionally as goal posts to permit all or a majority of the gatekeepers to approve a patchset in the absence of reviews by anyone else. > * maybe we should keep a regularly-updated scoreboard and award brownie > points for most active reviewers. You can add it to the newsletter statistics. > crazier ideas: > * are there other non-openafs communities that we could tap into? If > stackoverflow.com did code reviews, that would be awesome. > * could we tie in with some other shared reputation/reward system? (i.e. > earning stackoverflow credits, slashdot karma, bitcoins, 1 free support > incident from an openafs partner) > * rewards for marketing openafs to the outside world. This might draw in > fresh blood. > * frequent reviewers earn more votes on gerrit, with some type of > metamoderation to deter abuse. > * require a committer to review X patches, from a different person, for > each patchset that he commits.(X could be 1-3 based on what the group > decides) > * each reviewer gets X votes per patch that may be used to vote for > feature prioritization. The problem with these crazier ideas is that for the most part they assume that there is some pool of resources (aka money) that can be used to provide rewards. And feature prioritization is driven by the entities that are funding the proposed work as constrained by legal or other issues. The OpenAFS developer community is dominated by a handful of financially motivated contributors. See https://www.ohloh.net/p/openafs/analyses/latest for a nice graphical view of where the contributions come from. Your File System Inc. of course is funding myself, Derrick Brashear, Simon Wilkinson, Peter Scott, Rod Widdowson, and some of Marc Dionne and Matt Benjamin's contributions. Sine Nomine Associates funds Andrew Deason, Michael Meffie, and Tom Keiser. Those two entities are responsible for over 85% of the contributions in the last 12 months. This is of course the crux of the problem with OpenAFS. There simply isn't a pool of independent developers to pull from. Things were a lot easier a decade ago when there was lots of low hanging fruit to work on. Now most of the efforts are huge multi-month efforts with big architectural impact. Such projects only happen when someone is willing to write a very big check or take a huge out of pocket risk. Jeffrey Altman
signature.asc
Description: OpenPGP digital signature
