One of issues which came out of the really interesting discussion about code freeze was the problems of getting peer reviews completed. Having seen how that discussion evolved, I thought it was worth starting another.
To start off with... we are definitely improving! In 2011/12 we did 769 peer reviews, 2012/13 1539 and in the last 12 months 2200 peer reviews have been completed. Much more important than that is that the quality of reviews is improving (at least from my perspective as an integrator). Twelve months ago we were still integrating code which had not been peer reviewed on a regular basis; today it's a rarity. Moodle code has never been as throughly reviewed as it does today.
That said, we continue to have a massive backlog of issues waiting for peer review and we have an even more gigantic pile of patched issues which have not ever been looked at. This is bad, i'm sure you know, but to reiterate:
- The longer an issue is delayed before integration, the bigger the chance it won't be relevant or conflict
- Contributors tend to be much more able to respond to review comments when initially submitting code
- You can get a new contributor 'hooked' by reacting quickly to their patch (it worked for me!)
Here are some of the causes of this problem, as I see it:
- There is an incredible amount of code being created by the Moodle community and it's not as fun to review code as to write it!
- In Moodle HQ, we don't quite have the right processs/balance/prioritisation for managing peer reviews (in contrast to Integration Review, where we have a full-time team dedicated to the task). I don't think it is necessary/appropiate for the community to discuss this! I'm just acknowledging I don't think it is a perfect situation.
- There is a degree of expectation that Moodle HQ should be resonsible for all peer reviews.
- Developers don't feel knowledgeable enough about a code area to peer review many issues, so they ignore it.
- Some issues are so big/fundamental/controversial they get ignored because the peer reviewer wouldn't know the next step.
- Some issues are so in the wrong direction/quality they get ignored because it is quite difficult to give bad feedback.
Some thoughts of mine:
- It could be beneficial to have a way of seperating the big/controversial/hard issues away from smaller ones. With the goal of processing the smaller ones as fast as possible (and getting a small number waiting in the queue). Ideally we'd also tell the developer of the issue that their issue is a trickier one to review and that they would expect delays. My argument against this is that I'm not sure if this triaging would just be moving the problem around.
- Somehow I would like to empower you to peer review something you are uncomfortable with, especially if the issue has been waiting for a while. There are many areas of Moodle which are not well understood by many people and by leaving the issue to someone else we don't help solve that. It's better the assignee gets some feedback, even if you are not a domain expert. (Note: this is kind of what the Integration team have to do on some issues on a weekly basis )
What are your thoughts?