Skip to content

Commit a1c6dc2

Browse files
[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.
1 parent ccddd13 commit a1c6dc2

File tree

2 files changed

+14
-2
lines changed

2 files changed

+14
-2
lines changed

llvm/docs/CodeReview.rst

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,12 +142,18 @@ from specific performance tests), please explain as many of these up front as
142142
possible. This allows the patch author and reviewers to make the most efficient
143143
use of their time.
144144

145+
.. _lgtm_how_a_patch_is_accepted:
146+
145147
LGTM - How a Patch Is Accepted
146148
------------------------------
147149

148150
A patch is approved to be committed when a reviewer accepts it, and this is
149151
almost always associated with a message containing the text "LGTM" (which
150-
stands for Looks Good To Me). Only approval from a single reviewer is required.
152+
stands for Looks Good To Me).
153+
154+
Only approval from a single reviewer is required, unless the pull request
155+
has required reviewers. In which case, you must have approval from all of those
156+
reviewers.
151157

152158
When providing an unqualified LGTM (approval to commit), it is the
153159
responsibility of the reviewer to have reviewed all of the discussion and

llvm/docs/GitHub.rst

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,10 +138,16 @@ you won't encounter merge conflicts when landing the PR.
138138
collaborating with others on a single branch, be careful how and when you push
139139
changes. ``--force-with-lease`` may be useful in this situation.
140140

141+
Approvals
142+
---------
143+
144+
Before merging a PR you must have the required approvals. See
145+
:ref:`lgtm_how_a_patch_is_accepted` for more details.
146+
141147
Landing your change
142148
-------------------
143149

144-
When your PR has been accepted you can merge your changes.
150+
When your PR has been approved you can merge your changes.
145151

146152
If you do not have write permissions for the repository, the merge button in
147153
GitHub's web interface will be disabled. If this is the case, continue following

0 commit comments

Comments
 (0)