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

Brian Gesiak via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 27 10:06:46 PST 2019


modocache accepted this revision.
modocache added a comment.

Thanks for this! This LGTM but I have two non-blocking suggestions.



================
Comment at: llvm/docs/CodeReview.rst:67
+intrinsics), adding language extensions in Clang, and so on, require an RFC on
+`*-dev` first. For changes that promise significant impact on users and/or
+downstream code bases, reviewers can request an RFC (Request for Comment)
----------------
MaskRay wrote:
>   ``*-dev``
New contributors may not be able to glean that `*-dev` refers to one of a number of mailing lists. May I suggest changing this sentence slightly, to: "...and so on, require a "request for comments" (RFC) email to one of the LLVM project's `*-dev` mailing lists first."


================
Comment at: llvm/docs/CodeReview.rst:68
+`*-dev` first. For changes that promise significant impact on users and/or
+downstream code bases, reviewers can request an RFC (Request for Comment)
+achieving consensus before proceeding with code review. That having been said,
----------------
I really appreciate spelling out the "RFC" acronym here, since brand-new contributors may not be familiar with this term. If I could make one or two small suggestions:

  # "RFC" first appears in the sentence before this one, in "require an RFC on `*-dev` first." Maybe the parenthetical "(Request for Comment)" should appear there instead of in this sentence?
  # "RFC" doesn't appear in the LLVM lexicon. Maybe you could add it, either as part of this patch, or in a separate commit?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71916





More information about the llvm-commits mailing list