[llvm] [llvm][docs] Add Approvals section to GitHub guide (PR #113434)

[David Spickett via llvm-commits]

https://github.com/DavidSpickett created https://github.com/llvm/llvm-project/pull/113434

Based on feedback that when reading the document as a guide, it's odd that it skips right from updating the PR to merging it.

The section is a link to the existing Code Review guide's text on the topic.

I have updated that to mention required reviewers, which some subprojects do use (libcxx is one) but most don't.

Also we use the words "accepted" and "approved" interchangeably, so I've swapped one instance so it's consistent between paragraphs.

>From cc738a3f5061c07cf0db76066813dc55da86965e Mon Sep 17 00:00:00 2001
From: David Spickett <david.spickett at linaro.org>
Date: Wed, 23 Oct 2024 10:25:19 +0100
Subject: [PATCH] [llvm][docs] Add Approvals section to GitHub guide

Based on feedback that when reading the document as a guide, it's
odd that it skips right from updating the PR to merging it.

The section is a link to the existing Code Review guide's text
on the topic.

I have updated that to mention required reviewers, which some
subprojects do use (libcxx is one) but most don't.

Also we use the words "accepted" and "approved" interchangeably,
so I've swapped one instance so it's consistent between paragraphs.
---
 llvm/docs/CodeReview.rst | 8 +++++++-
 llvm/docs/GitHub.rst     | 8 +++++++-
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/llvm/docs/CodeReview.rst b/llvm/docs/CodeReview.rst
index 56798ae4faf0c4..15e3e018e11586 100644
--- a/llvm/docs/CodeReview.rst
+++ b/llvm/docs/CodeReview.rst
@@ -142,12 +142,18 @@ from specific performance tests), please explain as many of these up front as
 possible. This allows the patch author and reviewers to make the most efficient
 use of their time.
 
+.. _lgtm_how_a_patch_is_accepted:
+
 LGTM - How a Patch Is Accepted
 ------------------------------
 
 A patch is approved to be committed when a reviewer accepts it, and this is
 almost always associated with a message containing the text "LGTM" (which
-stands for Looks Good To Me). Only approval from a single reviewer is required.
+stands for Looks Good To Me).
+
+Only approval from a single reviewer is required, unless the pull request
+has required reviewers. In which case, you must have approval from all of those
+reviewers.
 
 When providing an unqualified LGTM (approval to commit), it is the
 responsibility of the reviewer to have reviewed all of the discussion and
diff --git a/llvm/docs/GitHub.rst b/llvm/docs/GitHub.rst
index ce4308022bf9f0..75cdfa6abe0218 100644
--- a/llvm/docs/GitHub.rst
+++ b/llvm/docs/GitHub.rst
@@ -138,10 +138,16 @@ you won't encounter merge conflicts when landing the PR.
   collaborating with others on a single branch, be careful how and when you push
   changes. ``--force-with-lease`` may be useful in this situation.
 
+Approvals
+---------
+
+Before merging a PR you must have the required approvals. See
+:ref:`lgtm_how_a_patch_is_accepted` for more details.
+
 Landing your change
 -------------------
 
-When your PR has been accepted you can merge your changes.
+When your PR has been apporoved you can merge your changes.
 
 If you do not have write permissions for the repository, the merge button in
 GitHub's web interface will be disabled. If this is the case, continue following