Merge Requests & Code Reviews

How we submit code and conduct code reviews

Merge Requests

We keep our MRs as small as possible. There is no hard limit on the number of changed files per MR. However, fewer files are generally easier to review and ensure fewer mistakes are made.
Small and focussed commits make it much easier to understand how the MR author arrived at their solution. They also encourage to carefully divide up our work.
Source branches should be deleted once the branch gets merged.
Merge requests should have documentation (either in the MR description or in a Notion ticket) that gives reviewers the necessary context for understanding the MR, for example:
- Links to other related MRs
- Links to design documents or RFCs
- Screenshots of any visual changes
- How to test the MR

Code reviews present a great opportunity to practice what we preach, share knowledge and improve the quality of our codebase.
One of Our Engineering Values is to Be Respectful. Code can and should be criticised, but with respect ✊
Pairing on a merge request is the preferred option to review.
If a comment thread gets too long, hop on a call to resolve it. Conflicts are always better resolved face-to-face than in writing where the potential for misinterpretation is high.
Review and comment on your own MRs to provide reviewers with context to why you might've made certain decisions and what other designs you may have tried but ultimately discarded.
Asking different engineers for reviews helps to share knowledge.
Only the reviewer may resolve their discussions in GitLab unless agreed otherwise. Comments should be resolved once the author of the MR and the reviewer have reached some agreement about whether the comments have been addressed.
If a reviewer wishes to suggest some changes, submitting an MR into the MR to show those changes would be a helpful addition to a review.
Using tools such as Loom are useful for providing some screen recordings or video explanations to support an MR or code review.