[PATCH] D41665: [Docs] Add Contributing page.

[Simon Dardis via Phabricator via llvm-commits]

sdardis added a comment.

Thanks for doing this, @fhahn.

A few small comments.



================
Comment at: docs/Contributing.rst:29
+the code base. If you are interested in fixing a bug, please create an account
+for the bug tracker and assign it to you, to let people know you are working on
+it.
----------------
"assign it to you" -> "assign it to yourself"


================
Comment at: docs/Contributing.rst:39
+Once you reproduced and fixed the bug, it is time to submit a patch. The patch
+should
+
----------------
Colon after should.


================
Comment at: docs/Contributing.rst:55
+patch touches.
+
+A reviewer usually accepts the patch with a `Looks good to me` or `LGTM`. Once
----------------
I suggest adding a short paragraph stating that reviewers may request changes, and that if you are uncertain on how provide test cases/documentation to ask for guidance.

The previous and following paragraphs IMHO jump from "how to submit" to "when you patch is accepted". A brief description of our process of having patches go through a cycle of changes required / repost with fixes should be included.


================
Comment at: docs/Contributing.rst:57
+A reviewer usually accepts the patch with a `Looks good to me` or `LGTM`. Once
+that is done the change can be committed. In case you do not yet have commit
+access, please let people know during the review and someone should commit it
----------------
"In case you do not yet have commit access" -> "If you do not have commit access"

This matches the Phabricator documentation we have, and I think it's a little more straight forward.


================
Comment at: docs/Contributing.rst:62
+Sometimes it can take a while until someone has time to review the patch.
+In that case you can ping the patch. The common courtesy ping rate is one week.
+Remember that you are asking for valuable time from other professional developers.
----------------
"In that case you can ping the patch. The common courtesy ping rate is one week."

->

"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."

My suggestion here is based off new contributors not necessarily knowing the fairly common terminology.



================
Comment at: docs/Contributing.rst:63
+In that case you can ping the patch. The common courtesy ping rate is one week.
+Remember that you are asking for valuable time from other professional developers.
+
----------------
"Remember" -> "Please remember"


https://reviews.llvm.org/D41665