Skip to content

SOP: GitLab Merge Request - Reviewer Role

Purpose

This guide provides a step-by-step workflow for reviewing merge requests in GitLab. It is designed for developers of all experience levels, from those new to Git/GitLab to seasoned contributors.

The review is a critical part of development for ensuring quality, stability, and adherence to project standards. Use the checklist in section Reviewer Checklist: What to Look For to ensure all critical aspects of the merge request have been evaluated before approval.

How to Conduct a Review in GitLab

What: The process of providing feedback and your official approval or request for changes on an MR.

Why: To prevent changes that are difficult to understand, catch diverging views early, ensure adherence to standards, and generally share knowledge.

How:

  1. Understand the Goal: Read the MR Description and linked Issue to understand what problem the code is trying to solve and why.
  2. Use the Changes Tab: Navigate to the Changes tab to view the file differences (the "diff").
  3. Start a Review (Recommended): When leaving your first comment on a line of code, select Start a Review instead of Add comment now. This saves your comments as pending until you are ready to submit the full review, sending the author only one notification.
  4. Leave Line Comments: Place comments directly on specific lines of code where you have questions or suggestions.
  5. Use Suggestions: For minor improvements (e.g., typos, formatting), use the "Suggest change" feature to propose the change directly.
  6. Submit the Review: Once you have finished reviewing, navigate back to the Overview tab or the Review summary.
  7. Select the outcome of your review:
    • Approve: The code is good to merge (provided other conditions are met).
    • Comment: General feedback without an explicit approval or change request.
    • Request changes: Blocks the MR from merging until the requested changes are addressed.
  8. (Optional but recommended) Leave a summary comment explaining your overall thoughts.

Tips:

  • Focus your feedback on the code, not the person. Assume the author has a positive intent.
  • Frame suggestions as questions ("What do you think about refactoring X?") rather than demands ("Change X to Y.").
  • Clearly explain the why behind a suggestion, not just the what.
  • Consider asking the author to break down the MR if it is too large. The quality of the review decreases with the size of change.

Reviewer Checklist: What to Look For

Use this checklist to ensure all critical aspects of the Merge Request have been evaluated.

Functionality & Requirements

  • Does the change meet all requirements outlined in the Issue/MR description?
  • Are all obvious edge cases (e.g., null input, error states, boundary conditions) handled appropriately?
  • Does the new feature or fix work correctly in the expected environment (local or staging)?

Code Quality & Style

  • Is the file structure clear, simple, and easy to understand?
  • Are variable, function, and class names descriptive and meaningful?
  • Does the content follow established project style guides (e.g., indentation, naming conventions)?
  • Is the logic clean and well-organized, avoiding unnecessary repetition or complexity?

Safety & Performance

  • Does the change introduce any obvious security vulnerabilities (e.g., SQL injection, exposed secrets, unchecked user input)?
  • Is there any inefficient logic, such as an expensive operation inside a loop or redundant database queries?
  • Are proper error handling and logging mechanisms in place (i.e., errors are caught, logged, and gracefully handled)?
  • For schema changes: Does the update maintain backward compatibility, or is the necessary migration documented?

Testing & Documentation

  • Are new or updated automated tests (unit/integration) included for the new logic?
  • Do the tests cover the core functionality and essential edge cases?
  • Are public-facing files (e.g., README) or complex code sections updated with clear comments or documentation?
  • Is the testing procedure outlined in the MR description reproducible and clear?

Additional Resources