[PATCH] D69353: Clean up the language for contributing

Wed Oct 23 12:52:10 PDT 2019

jeremybennett updated this revision to Diff 226172.
jeremybennett added a comment.

- More clean up of the language for contributing

Clarify needs for documentation patches, provide link to
	instructions for command line access to Phabricator, use more
	inclusive language on how to find a suitable reviewer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69353

Files:
  llvm/docs/Contributing.rst


Index: llvm/docs/Contributing.rst
===================================================================

--- llvm/docs/Contributing.rst
+++ llvm/docs/Contributing.rst
@@ -48,39 +48,61 @@
 
 How to Submit a Patch
 =====================
-Once you have a patch ready, it is time to submit it. The patch should:
-
-* include a small unit test
-* conform to the :doc:`CodingStandards`. You can use the `clang-format-diff.py`_ or `git-clang-format`_ tools to automatically format your patch properly.
-* not contain any unrelated changes
-* be an isolated change. Independent changes should be submitted as separate patches as this makes reviewing easier.
+Once you have a patch ready, it is time to submit it. For code patches you
+should:
+
+* include a small unit test.
+* conform to the :doc:`CodingStandards`.  You can use the
+  `clang-format-diff.py`_ or `git-clang-format`_ tools to automatically format
+  your patch properly.
+* not contain any unrelated changes.
+* be an isolated change. Independent changes should be submitted as separate
+  patches as this makes reviewing easier.
+
+For documentation patches:
+
+* there is no need for a unit test, although if the documentation includes a
+  code example, a unit test of the example would be appropriate.
+* there is not yet a documentation style guide, so aim to follow a similar
+  style to existing documentation.
+* use inclusive language.
+* not contain any unrelated changes.
+* be an isolated change. Independent changes should be submitted as separate
+  patches as this makes reviewing easier.
 
 To get a patch accepted, it has to be reviewed by the LLVM community. This can
 be done using `LLVM's Phabricator`_ or the llvm-commits mailing list.
 Please  follow :ref:`Phabricator#requesting-a-review-via-the-web-interface <phabricator-request-review-web>`
-to request a review using Phabricator.
-
-To make sure the right people see your patch, please select suitable reviewers
-and add them to your patch when requesting a review. Suitable reviewers are the
-code owner (see CODE_OWNERS.txt) and other people doing work in the area your
-patch touches. If you are using Phabricator, add them to the `Reviewers` field
-when creating a review and if you are using `llvm-commits`, add them to the CC of
-your email.
+to request a review using Phabricator via the web interface.  Alternatively
+:ref:`https://secure.phabricator.com/book/phabricator/article/arcanist_diff/`
+shows how to request a review using Phabricator via the command line.
+
+Please select suitable reviewers and add them to your patch when requesting a
+review. Suitable reviewers include the code owner for the area your patch
+affects.  There is a file named ``CODE_OWNERS.txt`` or ``CODE_OWNERS.TXT`` in
+the subdirectory for each project of the main ``llvm-project`` repository
+which lists the code owners for that project.  Other appropriate reviewers
+include people doing work in the area your patch touches - they will be
+posting discussions of their work on the mailing list.  If you are using
+Phabricator, add them to the `Reviewers` field when creating a review and if
+you are using `llvm-commits`, add them to the CC of your email.
 
 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.
+uncertain on how to provide test cases, documentation, etc., ask for guidance
+during the review. You can then 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`.  This can take a while - on average a review takes 3 or 4
+cycles.  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 a patch by responding to the email thread containing the
-patch, or the Phabricator review with "Ping." The common courtesy 'ping' rate
-is once a week. Please remember that you are asking for valuable time from other
-professional developers.
+review by responding to the email thread containing the patch, or the
+Phabricator review with a single line message "Ping."  The common courtesy
+'ping' rate is once a week.  Sometimes reviewers can be busy, and if you are
+finding difficulty with getting a patch reviewed after a couple of weeks, ask
+the code owner to help find alternative reviewers.
 
 
 Helpful Information About LLVM


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D69353.226172.patch
Type: text/x-patch
Size: 5065 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20191023/f4de6af8/attachment.bin>
