Skip to content

[docs] Update docs on code-review process #111735

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion llvm/docs/CodeReview.rst
Original file line number Diff line number Diff line change
Expand Up @@ -150,14 +150,20 @@ 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.

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

If additional feedback is provided after acceptance (by the same reviewer or
another), the author should use their best judgement in deciding whether that
feedback can be incorporated into the change without comment (say a typo) or
requires further review discussion. More substantial comments (e.g., about the
design) will usually require further discussion. If unsure, ask the reviewer.

Note that, if a reviewer has requested a particular community member to review,
and after a week that community member has yet to respond, feel free to ping
the patch (which literally means submitting a comment on the patch with the
Expand Down
18 changes: 6 additions & 12 deletions llvm/docs/Contributing.rst
Original file line number Diff line number Diff line change
Expand Up @@ -102,21 +102,15 @@ will not be able to select reviewers in such a way, in which case you can still
get the attention of potential reviewers by CC'ing them in a comment -- just
@name them.

A reviewer may request changes or ask questions during the review. If you are
uncertain on how to provide test cases, documentation, etc., feel free to ask
for guidance during the review. Please address the feedback and re-post an
updated version of your patch. This cycle continues until all requests and comments
have been addressed and a reviewer accepts the patch with a `Looks good to me` or `LGTM`.
Once that is done the change can be committed. If you do not have commit
access, please let people know during the review and someone should commit it
on your behalf.

If you have received no comments on your patch for a week, you can request a
review by 'ping'ing the GitHub PR with "Ping". The common courtesy 'ping' rate
is once a week. Please remember that you are asking for valuable time from other
professional developers.
is once a week. Please remember that you are asking for valuable time from
other professional developers. Finally, if you do not have commit access,
please let people know during the review and someone should commit it on your
behalf once it has been accepted.

For more information on LLVM's code-review process, please see :doc:`CodeReview`.
For more information on LLVM's code-review process, please see
:doc:`CodeReview`.

.. _commit_from_git:

Expand Down
Loading