f_mr_review
Last updated: 2024-09-17 21:09:18.587669 File source: link on GitLab
Merge Request (MR) Review Process
Peer Code Review System
To scale MR reviews, a peer code review system is adopted. When opening an MR, the developer should ideally choose a peer reviewer based on:
familiarity with the requirements;
knowledge of the module or code;
relevant skills;
prior experience.
It is also possible to take advantage of different time zones by assigning default reviewers in regions like APAC, EMEA, and Brazil to ensure real-time coordination and faster development. This approach improves review quality, fosters mentoring relationships among developers, and boosts long-term development velocity.
MR Labels and Priority
When opening an MR, the creator must tag it with one of the following labels:
domain::core platform
domain::platform
domain::application
domain::documentation
domain::infrastructure
domain::testing
Additionally, use the Priority::High tag if the MR is blocking other developers.
If the code is not ready for review, it must be marked as Draft.
Peer Review Focus Areas
Peers must review MRs with three main focuses:
Functionality: Verify if the code meets the functional requirements outlined in the issue.
Code quality: Review for adherence to established standards, code quality, bugs, security vulnerabilities, duplication, performance, and maintainability.
Testing: Ensure tests for new functionalities are created with good coverage and quality. For bug fixes, ensure the unit test is updated to specifically cover the condition that was fixed.
In addition to peer reviews, some MRs will also undergo an architectural or conceptual review. This review can be requested by either the MR creator or the peer reviewer.
Approval and Merging
The approval process differs depending on the target branch:
An MR opened to the
mainbranch requires approval from a reviewer on the development team.An MR opened to a
releasebranch requires approval from a reviewer and a member of the security team.
Each repository has an ultimate responsible for merging code, but this person will not conduct in-depth reviews of all MRs. Peer reviews will be managed by other developers, with the responsible person doing a final check before merging. Going forward, the responsible should only be assigned as a reviewer if it is expected an actively involvement in the review.
Mote: See this documentation for details related to the branching strategy.
Responsibilities
The peer reviewer must analyze the status of each pipeline stage and, if approving an MR with warnings or failures in any stage, provide an explanation in a comment. For example, the reviewer can create a new issue to address the problem and link it in the comment.
Similarly, if a comment remains unresolved in the MR, a follow-up issue can be created, linked in the comment, and the MR can still be approved.
The reviewer must comment on the MR, even if it is just a "Looks Good To Me" (LGTM), to indicate that the changes have been reviewed and there are no objections or further comments.
The MR creator must promptly address any comments until the reviewer approves or closes the MR.
Last updated