[llvm] 41248b5 - [docs] Update docs on code-review process (#111735)

via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 5 23:39:46 PST 2024


Author: Andrzej WarzyƄski
Date: 2024-11-06T07:39:43Z
New Revision: 41248b598b8b18febc62ea61938870def2421126

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

LOG: [docs] Update docs on code-review process (#111735)

Clarify expectations for handling new comments post-LGTM but pre-commit.

This change aims to standardize expectations when new comments are added
after a patch has received LGTM but before it has been committed.
Currently, approaches to this vary, and this update seeks to clarify
best practices.

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/llvm/docs/CodeReview.rst b/llvm/docs/CodeReview.rst
index c3ca82d061f002..4cfd91435247cd 100644
--- a/llvm/docs/CodeReview.rst
+++ b/llvm/docs/CodeReview.rst
@@ -156,7 +156,7 @@ 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
+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.,
@@ -164,6 +164,12 @@ approved. If unsure, the reviewer should provide a qualified approval, (e.g.,
 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

diff  --git a/llvm/docs/Contributing.rst b/llvm/docs/Contributing.rst
index fd7c0b57c3953a..38c05811828642 100644
--- a/llvm/docs/Contributing.rst
+++ b/llvm/docs/Contributing.rst
@@ -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:
 


        


More information about the llvm-commits mailing list