Git Course 0%

Code review on GitLab

Intermediate GitLab UI ≈ 10 min

Before this lesson

What you will learn

  • The mechanics: threads, resolving, suggestions, approving
  • How to write a review comment that helps rather than annoys
  • What a non-developer can usefully review, and how to say so

After this lesson you can

  • I can review a colleague's merge request usefully, and respond well to a review of mine

Why this matters

Review is the moment a change stops being one person's opinion. As a non-developer you will be reviewed constantly and, more often than you expect, asked to review: documentation, wording in the interface, error messages, release notes. The mechanics take five minutes to learn; the manner takes a little care and is what makes people want you on their merge requests.

The mechanics

On the Changes tab, hovering over a line shows a speech-bubble icon. Clicking it opens a comment on that line, which becomes a thread.

Action What it does
Comment Posts the thread immediately
Start a review Holds your comments until you submit them together, so the author gets one notification instead of twelve
Add to review Adds another comment to the batch you are building
Submit review Posts them all, with an optional summary and a verdict
Resolve thread Marks a thread as settled; usually the author does this after acting
Approve Records that you are happy for it to merge

Prefer Start a review over individual comments on anything longer than a one-line fix: it is much kinder to the author's notifications and lets you revise a comment before anyone sees it.

Whether a merge request needs all threads resolved before merging is a project setting (lesson 9.5); many teams require it, which makes resolving a thread a small act of bookkeeping rather than a judgement.

Suggestions

For anything that is "this word should be that word", write a suggestion rather than a sentence describing the change. Select the lines, click the comment icon, then the Insert suggestion button (or type the suggestion fence by hand):

Markdown
```suggestion:-0+0
Yes. Trailguide is free for everyone, including commercial use.
```

The author sees a diff and an Apply suggestion button. This is the single most useful review skill for documentation work: unambiguous, one click, correctly attributed to you in the commit.

Writing a comment that helps

Instead of Write
"This is wrong." "This says 3.9, but the pipeline pins 3.12 — should it be 3.12?"
"Reword this." A suggestion with the wording you mean
"Why did you do it this way?" "What made you choose X over Y? I ask because…"
Twelve separate comments One review, batched, with a summary

Three habits that make a reviewer easy to work with:

  • Say what is optional. Prefix opinions with "nit:" or "non-blocking:" so the author knows what must change and what is a preference.
  • Praise the good bits, briefly. One line is enough and it changes how the rest reads.
  • Review promptly or say when you can. A merge request waiting two days is a person blocked for two days.

Being reviewed

  • Read the whole review before replying. Half the comments often answer each other.
  • Answer every thread, even if only "done" or "kept as is, because…". Silence reads as ignoring.
  • Push fixes as new commits rather than amending, so the reviewer can see what changed since they looked. The team may squash on merge anyway (lesson 6.6).
  • Do not take it personally, and do not write as though the other person might. Review comments are about the text, not the author.
  • Re-request review by re-assigning the reviewer once you have addressed everything; that sends a fresh to-do item.

Reviewing as a non-developer

You are qualified to review more than you think, and unqualified for some of it, and saying which is which is the professional move.

You can usefully check Say so like this
Wording, clarity, grammar, tone Just comment or suggest
Whether the docs match what the change actually does "The code now defaults to 10 but the guide still says 5"
Whether an error message will make sense to a user "A user seeing this will not know what 'nil ref' means"
Whether the change is documented at all "Should this be in the changelog?"
Steps a user follows, by following them "I followed these on Windows and step 3 failed"

For the parts you cannot judge, say so plainly: "I reviewed the documentation only; the Python needs someone else." That is a useful review, and it prevents an approval being read as more than it was.

How to do it

Review happens on the platform, but two commands help you review properly by looking at the branch locally:

Terminal
$ git fetch
$ git switch docs/12-windows-install-steps

For documentation, that lets you build or preview the pages; for a lab or a guide, it lets you actually follow the steps, which is the most valuable review a writer can give.

Common mistakes

  • Twelve separate comments instead of one batched review, so the author gets twelve notifications.
  • "Reword this" without saying how. Use a suggestion.
  • Approving without saying what you looked at, which implies more than you checked.
  • Leaving threads unanswered as the author, so the reviewer cannot tell what was considered.
  • Amending and force-pushing mid-review, which erases the commits the reviewer's comments refer to (lesson 6.7).
  • Reviewing only the diff when the change is a set of instructions. Follow them.

Try it yourself

Goal: give a real review, including a suggestion.

  1. On your practice project, create a branch with a deliberate small error in docs/faq.md: a typo and a factual mistake, for example the wrong Python version.
  2. Push it and open a merge request.
  3. Review your own merge request: on Changes, start a review, leave a suggestion fixing the typo, and a comment on the factual error explaining what is wrong and why.
  4. Submit the review, then switch hats: apply the suggestion, fix the fact, answer the thread and resolve it.
  5. Look at the merge request's activity: the suggestion became a commit attributed to you as suggester.

Expected result: one batched review, one applied suggestion committed automatically, one thread answered and resolved.

Show solution

Reviewing your own merge request is artificial but it is the only way to see both sides of the mechanics in ten minutes. The part worth noticing is step 5: an applied suggestion credits the reviewer, which is why suggestions are the polite way to fix someone's wording rather than editing their branch directly.

Check yourself

1. You have five comments to make on one merge request. What is the considerate way?
2. A wording change is needed on one line. What is the most useful comment?
3. You reviewed only the documentation part of a mixed change. What should you do?

Key terms

Code review Merge request (MR) Reviewer