title: TPA-RFC-79: General merge request workflows
costs: N/A
approval: TPA
affected users: TPA
deadline: 1 week, 2025-02-11
status: proposed
discussion: emailSummary: how to use merge requests, assignees, reviewers, draft and threads on GitLab projects
Background
There seems to be different views on how to use the various merge request mechanisms in GitLab to review and process merge requests (MR). It seems to be causing some confusion (at least for me), so let's see if we can converge over a common understanding.
This document details the various mechanisms that can be used in merge requests and how we should use merge requests themselves.
Assignee
The "author" of a merge request, typically the person that wrote the code and is proposing to merge it in the codebase, but it could be another person shepherding someone else's code.
In any case, it's the person that's responsible for responding for reviews and making sure the merge request eventually gets dealt with.
A person is assigned to a merge request when it's created. You can reassign a merge request if someone is available to actually work on the code to complete it.
For example, it's a good idea to reassign your MR if you're leaving on vacation or you're stuck and want to delegate the rest of the work to someone else.
Reviewers
Reviewers are people who are tasked with reviewing a merge request, obviously. Those are typically assigned by the assignee, but could self-elect to review a piece of code they find interesting.
You can request a review from a specific person with the
/assign_reviewer @foo quick action or the "reviewer" menu.
Whenever you are reviewing your fellow's work, be considerate and kind in your review. Assume competence and good will, and demonstrate the same. Provide suggestions or ideas for problems you discover.
If you don't have time to review a piece of code properly, or feel out of your depth, say so explicitly. Either approve and drop a "looks good to me!" (LGTM!) as a comment, or reassign to another reviewer, again with a comment explaining yourself.
It's fine to "LGTM" code that you have only given a cursory glance, as long as you state that clearly.
Drafts
A merge request is a "draft" when it is, according to its author, still a "work in progress". This signals actual or possible reviewers that the merge request is not yet ready to be reviewed.
Obviously, a draft MR shouldn't be merged either, but that's implicit: it's not because it's draft, it's because it hasn't been reviewed (and then approved).
The "draft" status is the prerogative of the MR author. You don't mark someone else's MR as "draft".
You can also use checklists in the merge request descriptions to outline a list of things that still need to be done before the merge request is complete. You should still mark the MR as draft then.
Approval and threads
A MR is "approved" when a reviewer has reviewed it and is happy with it. When you "approve" a MR, you are signaling "I think this is ready to be merged".
If you do not want a MR to be merged, you add a "thread" to the merge request, ideally on a specific line of the diff, outlining your concern and, ideally, suggesting an improvement.
(Technically, a thread is a sort of "comment", you actually need to "start a thread", which makes one "unresolved thread" that then shows up in a count at the top of the merge request in GitLab's user interface.)
That being said, you can actually mark a MR as "approved" even if there are unresolved threads. That means "there are issues with this, but I'm okay to merge anyways, we can fix those later".
Those unresolved threads can easily be popped in a new issue through the "three dots" menu in GitLab.
Either way, all threads SHOULD be resolved when merging, either by marking them as resolved, or by defering them in a separate issue.
You can add unresolved threads on your own MR to keep it from being merged, of course, or you can mark your own MR as "draft", which would make more sense. I do the former when I am unsure about something and want someone else to review that piece: that way, someone can resolve my comment. I do the latter when my MR is actually not finished, as it's not ready for review.
When and how to use merge requests
You don't always need to use all the tools at your disposal here. Often, a MR will not need to go through the draft stage, have threads, or even be approved before being merged. Indeed, sometimes you don't even need a merge request and, on some projects, can push directly to the main branch without review.
We adhere to Martin Fowler's Ship / Show / Ask branching strategy which is, essentially:
Ship: no merge request
Just push to production!
Good for documentation fixes, trivial or cosmetic fixes, simple changes using existing patterns.
In this case, you don't use merge requests at all. Note that some projects simply forbid this entirely, and you are forced to use a merge request workflow.
Not all hope is lost, though.
Show: self-approved merge requests
In this scenario, you make a merge request, essentially to run CI but also allowing some space for conversation.
Good for changes you're confident on, sharing novel ideas, and scope-limited, non-controversial changes. Also relevant for emergency fixes you absolutely need to get out the door as soon as possible, breakage be damned.
This should still work in all projects that allow it. In this scenario, either don't assign a reviewer or (preferably) assign yourself as your own reviewer to make it clear you don't expect anyone else's review.
Ask: full merge request workflow
Here you enable everything: not only make a MR and wait for CI to pass, but also assign a reviewer, and do respond to feedback.
This is important for changes that might be more controversial, that you are less confident in, or that you feel might break other things.
Those are the big MRs that might lead to complicated discussions! Remember the reviewer notes above and be kind!