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

[Hal Finkel via Phabricator via llvm-commits]

hfinkel marked 45 inline comments as done.
hfinkel added a comment.

I would like to thank everyone who provided feedback on this patch. I apologize for the slow turn-around time on this. I'll upload a revised version that, I believe, addresses all feedback.



================
Comment at: llvm/docs/CodeReview.rst:99
+the author can make all of the changes at once. If a patch will require
+multiple steps prior to approval (e.g., splitting, refactoring, posting data
+from specific performance tests), please explain as many of these up front as
----------------
hubert.reinterpretcast wrote:
> jhenderson wrote:
> > e.g., -> e.g.
> > 
> > Same goes for all other instances of this pattern. Alternatively, consider using "for example,"
> The use with the comma is consistent with the use and description in the MLA Handbook, 6th ed. My understanding is that the comma is customary in North American usage.
That's correct. This is, unfortunately, a traditional style difference between different parts of the world.


================
Comment at: llvm/docs/CodeReview.rst:207
+  asking for valuable time from other professional developers.
+* Ask for help on IRC. Developers on IRC will be able to either help you
+  directly, or tell you who might be a good reviewer.
----------------
Mordante wrote:
> MaskRay wrote:
> > mehdi_amini wrote:
> > > "Ask for help on IRC or Discord."?
> > From https://lists.llvm.org/pipermail/llvm-dev/2019-November/136880.html , people are concerned about Discord's user policy. Not mentioning it here (before we reach a consensus) should be fine.
> Add a link to the IRC channel?
It's on the front page along with the mailing lists. If we're going to have links to the channel, I'd rather have a page on it so we can centralize that information (instead of duplicating it in various places throughout the documentation).


================
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.
-
----------------
hubert.reinterpretcast wrote:
> Did the "write access"/"commit access" requirement survive?
Thanks. I added it back in the LGTM section.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71916