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

via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 31 08:24:36 PDT 2024


Author: David Spickett
Date: 2024-10-31T15:24:33Z
New Revision: a1c6dc223ee1eedc049890676992b508ebd6b623

URL: https://github.com/llvm/llvm-project/commit/a1c6dc223ee1eedc049890676992b508ebd6b623
DIFF: https://github.com/llvm/llvm-project/commit/a1c6dc223ee1eedc049890676992b508ebd6b623.diff

LOG: [llvm][docs] Add Approvals section to GitHub guide (#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.

Added: 
    

Modified: 
    llvm/docs/CodeReview.rst
    llvm/docs/GitHub.rst

Removed: 
    


################################################################################
diff  --git a/llvm/docs/CodeReview.rst b/llvm/docs/CodeReview.rst
index 2024773dd8b24f..c3ca82d061f002 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..d785d9da9a7f48 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 approved 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


        


More information about the llvm-commits mailing list