Abstract Wikipedia team/Git workflow

More eyes on code review

At the Abstract Wikipedia team we have implemented a code-review workflow called "Mode eyes on code review" to ensure that at least two team members get to review one patch before it's merged. The goal of this system is to help with knowledge sharing, help navigate delicate areas of the code, and resolve some conflicts between author and first reviewer by adding a third party.

After an experimental one-month implementation (see the experiment design and results) the whole team agreed to implement this as a permanent practice.

We currently have repositories sitting on both Gerrit and on GitLab. These are the protocols we follow for each:

More eyes on Gerrit

To review and merge patches from our extension repository extensions/WikiLambda:

1. When an AW engineer wants a patch merged, they will tag two reviewers from the AW team in Gerrit
2. It is recommended, but not required that one person is very familiar with the area of the code-based covered in the patch and the other is not
3. The two reviewers will do their best to review the patch within one working day.
4. The first of the two reviewers will give the patch a maximum of +1 with a comment on how confident they feel about merging the patch.
5. If the first reviewer has given a +1 and the second reviewer also feels confident in merging the patch, they will give it a +2.
6. If someone is added as a reviewer and they know they will not be able to review the patch in one working day, they will remove themselves as a reviewer and tag someone else as a reviewer.

Design review

If the patch consists on a front-end change that significantly affects design, the author should tag an additional reviewer from the AW design team who can inspect and approve the new changes.

As a code reviewer: if you notice that the patch affects design and/or you notice the design team is tagged, make sure that they have noticed and approved the changes before merging.

More eyes on GitLab

To review and merge patches from function-orchestrator, function-evaluator and function-schemata, which are on GitLab:

1. When an AW engineer wants a patch merged, they will tag one person as an assignee and one person as a reviewer:
   • An assignee is the person responsible for accepting the merge; there can only be one assignee.
   • Reviewers are additional people that can follow, comment and approve a merge request; there can be many reviewers.
2. It is recommended, but not required that one person is very familiar with the area of the code-based covered in the patch and the other is not.
3. The assignee and the reviewer will do their best to review the patch within one working day.
4. The first person who reviews the patch (whether they are marked as assignee or reviewer) will click the "Approve" button after they've completed the review and they feel confident about merging the patch.
5. As a courtesy, if the person that first approves the patch is marked as an assignee, they will move the remaining reviewer to assignee.
6. If the second person to review the patch is also confident about it, will finally click the button "Merge".
7. If someone is added as a reviewer or assignee and they know they will not be able to review the patch in one working day, they will remove themselves and tag someone else.