On 19-Apr-09, at 4:38 PM, Karoly Negyesi wrote:
we always had too few reviewers and now the entry barrier is even harder as you need to do meaningful code reviews all the time
A significant issue I've encountered in participating with core patches is the time required to synthesize hundreds of comments into a cohesive overview of where the issue is at and what the next steps are. Of course, most of the comments will be valid points of discussion, but the barrier to entry can be quite high due to the initial time involved to get familiar with the issue. One idea to help simply patch reviews would be to add single checklist of review tasks for an issue. CNR is too vague of a status to be meaningful in many cases. When a patch is set to CNR for the first time, review items (such as those at http://drupal.org/patch/review) could be added to the issue automatically. One item would be "passing the test bot". Each checkbox could have associated comment ID / user name fields, so it would be easy to say "this code has been benchmarked at comment #1234". Those more involved with the issue would need to be able to add additional tasks as well. Code isn't set to RTBC until all of the tasks have been marked as done. I think this would help get more individuals reviewing patches. For example, perhaps one of the tasks for UI related patches would be "Needs screenshot of changes". A potential reviewer could search for all CNR issues where that task needs to be done, and do them all at once. Developers have specific specialities, and as-is it's not incredibly easy for new reviewers to find the issues where they can contribute the most. --Andrew