Skip to content

Commit a9f5488

Browse files
committed
[docs] Update docs on code-review process
Clarify what to do in cases where multiple reviewers commented on a patch, but only one reviewer explicitly accepted. This is mostly to standardize what the expectations are as folks tend to approach this differently.
1 parent ac4bd74 commit a9f5488

File tree

1 file changed

+10
-3
lines changed

1 file changed

+10
-3
lines changed

llvm/docs/Contributing.rst

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -105,9 +105,16 @@ 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 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
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
111118
access, please let people know during the review and someone should commit it
112119
on your behalf.
113120

0 commit comments

Comments
 (0)