Note to myself: Make sure that https://www.mediawiki.org/wiki/Gerrit/Tutorial#How_code_is_reviewed_in_Gerrit only covers technical, Gerrit specific, review aspects and is different from "generic" https://www.mediawiki.org/wiki/Gerrit/Code_review

Status update: I failed to complete this personal quarterly goal in Apr-Jun 2016 - we only made progress in related areas.

When preparing to work on T129068: Improve code contribution guidelines for patch authors (my other personal quarterly goal) I realized that I hadn't taken into account yet another obstacle to solve first which took quite some additional time: T134256: Make Wikimedia's Git/Gerrit documentation less horrible and less scattered.

Hence I've added this task to T131915: Technical Collaboration quarterly goals for July - September 2016.

Reviewers' CR comments considered not useful by contributors: [...] reviewers asking questions to understand the implementation

On the other hand, if the reviewer doesn't understand it then they probably won't (and shouldn't) merge it. Reviewer confusion can also indicate an opportunity for improved inline comments or a restructuring of the contribution to make it easier to follow.

18 edits later, https://www.mediawiki.org/wiki/Gerrit/Code_review looks a bit better.

Still to do regarding that wiki page:

• https://www.mediawiki.org/wiki/Gerrit/Code_review#Review_checklist is too long, too detailed, and not a structured approach. Nobody reads that.
  • Think and decide about a structured code review approach: https://www.mediawiki.org/wiki/User:AKlapper_(WMF)/Code_Review#Unstructured_review_approach
  • Document what kind of comments are considered helpful: https://www.mediawiki.org/wiki/User:AKlapper_(WMF)/Code_Review#Unhelpful_reviewer_comments

Performed some more edits on the docs for reviewers. Regarding my personal involvement, I consider the documentation part finished.

I would like to reassign this task to Engineering to lead efforts to have teams follow those guidelines, establish a routine of going through unreviewed patches, and other iterative improvements.

In T129067#2582754, @Aklapper wrote:

Performed some more edits on the docs for reviewers. Regarding my personal involvement, I consider the documentation part finished.

Very good! This was a long way, thank you.

I would like to reassign this task to Engineering to lead efforts to have teams follow those guidelines, establish a routine of going through unreviewed patches, and other iterative improvements.

Since the theoretical agreement and the documentation have been completed, I think it would be better to resolve this task and then discuss with @greg / ReleaseEngineering / Technology & Product what are the next steps that they should lead because our time as leaders here has come to an end.

We have two open tasks that could host this discussion:

• T78768: Agree on and implement actions to prioritize code review of patches submitted by volunteers (already defined as parent / followup task)
• T113707: Decision on Wikimedia Foundation service-level agreement on code review times

For what it's worth, I think T78768: Agree on and implement actions to prioritize code review of patches submitted by volunteers and T113707: Decision on Wikimedia Foundation service-level agreement on code review times are too specific to host the more general discussion.

I would prefer to see a phab task that spans code review of both staff and volunteers (CR is an ongoing pain point even within teams), and focuses on the overall quality of code reviews, rather than just timeliness. The two issues above could be subtasks or just related tasks.

I have no strong opinion about how to organize the next steps in Phabricator. I do think it is useful to keep T78768 with a specific focus on volunteer developers. As much room for improvement there is in code review between professional developers, they still have many ways to communicate and discuss with each other that most (new) volunteer developers don't have access to or clue about.