Page MenuHomePhabricator

Review and refine the Code Review Office Hours model of engagment
Closed, DeclinedPublic

Description

During the Code Review work group meetings we discussed the possible revival of the Code Review Office Hours engagement model. One of the prospective changes would be to have them be targeted to a specific area of code in order to encourage participation from both patch submitters and appropriate code reviewers.

Desired outcomes:

  • Getting the right people together to review code synchronously
  • Identifying approaches to prioritization/target selection
  • ...

Previously:

Event Timeline

Some ideas that might help with participation:

For each event:

  • Post to mailing lists in advance detailing the areas we plan to focus on and inviting participation
  • Email patch submitters and reviewers directly?
  • Have more than one time slot to accommodate global participation

Ideas for targeted code review:

  • Focus on projects with a lot of outstanding patches, especially where there are a lot of low hanging fruit.
  • Solicit volunteers to nominate patches for code review. This worked previously, in fact I think that accounted for the majority of patches that I merged during the old code review office hours.
  • Team or stewardship based - arrange for code stewards to be available to work through the backlog, with the help of release engineering and any volunteers who choose to participate.

There was some discussion (in T173770: Code Review Hours advertised but not taking place? and related places) about realtime vs. async. On one hand real-time discussion (IRC instead of email or similar) is better for engaging new developers, and better for motivating reviewers to show up and actually do something. On the other hand reviewing and testing a nontrivial patch takes time and as such is not very suitable for a timeboxed meeting.

I wonder if it would make more sense to rebrand it as patch triage? There are elements of code review that are fast and very useful for a new developer - making sure the patch has decent documentation (summary or linked task) explaining the intent and need, that the patch has (or doesn't need) community consensus and product owner support, that it's a good idea to do on a high level, that the relevant people are assigned as reviewers (ie. the first and to some extent the second of the three phases of code review). The final review and marge could then happen later async via some different mechanism (cf. T200987: Set up volunteer code review queue).

Thanks for sharing your thoughts @Tgr, I like the idea of code review triage and I think that's a useful way to frame things.

mmodell triaged this task as Medium priority.Aug 6 2019, 2:17 PM

I'd like to propose a topic for the first patch triage: Extension Registration. There are a ton of patches: https://gerrit.wikimedia.org/r/#/q/extension+registration+is:reviewed+status:open

I was inspired by the core platform team "clinic duty" initiative, and I'd like to organize something similar for code review.

A code-review clinic duty could be hosted by Engineering Productivity, or a subteam. Would ideally have people from QTE and RelEng involved, and host temporary team members from other teams who would occasionally join us and spend either a day or a full sprint working on code reviews and paying down technical debt.

mmodell changed the task status from Open to Stalled.Apr 20 2020, 6:46 PM