[PATCH] D71916: High-Level Code-Review Documentation Update

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: llvm/docs/CodeReview.rst:146
+Reviewers may request certain aspects of a patch to be broken out into separate
+patches for independent review, and also, reviewers may accept a patch
+conditioned on the author providing a follow-up patch addressing some
----------------
Mordante wrote:
> Maybe change `review, and also, reviewers` to `review. Reviewers` in order to improve the readability?
If breaking the sentence, then please retain "also": `review. Reviewers may also`.


================
Comment at: llvm/docs/CodeReview.rst:149
+particular issue or concern (although no committed patch should leave the
+project in a broken state). Reviewers can also accept a patch conditioned on
+the author applying some set of minor updates prior to committing, and when
----------------
For variety, I would suggest using something other than "also".


================
Comment at: llvm/docs/CodeReview.rst:216
+favor for someone else.  Note that anyone is welcome to review and give feedback
+on a patch, but patches should be approved only consistent with the policy above.
+
----------------
I believe this should be the adverb, "consistently", applying to "approved" as a verb. Alternatively: `, but approval of patches should be consistent with the policy above`.


================
Comment at: llvm/docs/DeveloperPolicy.rst:163
-favor for someone else.  Note that anyone is welcome to review and give feedback
-on a patch, but only people with Subversion write access can approve it.
-
----------------
Did the "write access"/"commit access" requirement survive?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71916/new/

https://reviews.llvm.org/D71916