propose TPA-RFC-79: merge request workflows

policy.md

@@ -33,6 +33,7 @@ the Git repository for this wiki, run the command:
 
 * [TPA-RFC-77: Puppet merge](policy/tpa-rfc-77-puppet-merge)
 * [TPA-RFC-78: Dangerzone retirement](policy/tpa-rfc-78-dangerzone-retirement)
+* [TPA-RFC-79: General merge request workflows](policy/tpa-rfc-79-general-merge-request-workflows)
 
 ## Standard
 

policy/tpa-rfc-79-general-merge-request-workflows.md

+---
+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: email
+---
+
+Summary: how to use merge requests, assignees, reviewers, draft and
+threads on GitLab projects
+
+[[_TOC_]]
+
+# 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:
+
+ [Martin Fowler's Ship / Show / Ask]: https://martinfowler.com/articles/ship-show-ask.html
+
+## 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!