Skip to content

Commit 3714f26

Browse files
committed
fixup! [docs] Update docs on code-review process
Revert changes in llvm/docs/Contributing.rst, update llvm/docs/CodeReview.rst instead.
1 parent a9f5488 commit 3714f26

File tree

2 files changed

+8
-11
lines changed

2 files changed

+8
-11
lines changed

llvm/docs/CodeReview.rst

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,14 +150,18 @@ almost always associated with a message containing the text "LGTM" (which
150150
stands for Looks Good To Me). Only approval from a single reviewer is required.
151151

152152
When providing an unqualified LGTM (approval to commit), it is the
153-
responsibility of the reviewer to have reviewed all of the discussion and
153+
responsibility of the reviewer to have reviewed all of the prior discussion and
154154
feedback from all reviewers ensuring that all feedback has been addressed and
155155
that all other reviewers will almost surely be satisfied with the patch being
156156
approved. If unsure, the reviewer should provide a qualified approval, (e.g.,
157157
"LGTM, but please wait for @someone, @someone_else"). You may also do this if
158158
you are fairly certain that a particular community member will wish to review,
159159
even if that person hasn't done so yet.
160160

161+
If new comments are posted after the patch has been approved (but not yet
162+
merged), these need to be addressed and all new changes have to be reviewed and
163+
approved by a reviewer.
164+
161165
Note that, if a reviewer has requested a particular community member to review,
162166
and after a week that community member has yet to respond, feel free to ping
163167
the patch (which literally means submitting a comment on the patch with the

llvm/docs/Contributing.rst

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -105,16 +105,9 @@ get the attention of potential reviewers by CC'ing them in a comment -- just
105105
A reviewer may request changes or ask questions during the review. If you are
106106
uncertain on how to provide test cases, documentation, etc., feel free to ask
107107
for guidance during the review. Please address the feedback and re-post an
108-
updated version of your patch. This cycle continues until all requests and
109-
comments have been addressed and the reviewer accepts the patch with a `Looks
110-
good to me` or `LGTM`. With multiple active reviewers (i.e. reviewers who left
111-
comments), it is good practice to seek `LGTM` from all of them. Alternatively,
112-
when e.g. some reviewers go radio silent and you are blocked:
113-
* any reviewer can confirm that all comments from other reviewers have been
114-
addressed and the change is ready to land, or
115-
* if you decide not to wait for explicit `LGTM` from other reviewers, please
116-
leave a short justification.
117-
Once a change has been approved, it can be committed. If you do not have commit
108+
updated version of your patch. This cycle continues until all requests and comments
109+
have been addressed and a reviewer accepts the patch with a `Looks good to me` or `LGTM`.
110+
Once that is done the change can be committed. If you do not have commit
118111
access, please let people know during the review and someone should commit it
119112
on your behalf.
120113

0 commit comments

Comments
 (0)