[PATCH] D129255: [docs] Move code contribution from GettingStarted.rst to Contributing.rst

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers accepted this revision.
nickdesaulniers added inline comments.


================
Comment at: llvm/docs/Contributing.rst:57-61
 * 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.
+* has a single commit (unless stacked on another Differential), up-to-date with the upstream ``origin/main`` branch, and doesn't contain merges
----------------
Consider adding punctuation to your added point, then making the whole bulleted list consistent.


================
Comment at: llvm/docs/Contributing.rst:91
 
-
-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#phabricator-reviews <phabricator-reviews>`
-to request a review using Phabricator.
+We don't currently accept GitHub pull requests, and you'll need to send patches
+via :ref:`Phabricator#phabricator-reviews <phabricator-reviews>`.
----------------
Consider adding something to denote we also no longer use the mailing list?

`and the llvm-commits mailing list is deprecated`?


================
Comment at: llvm/docs/Contributing.rst:136-138
+  % git checkout branch-with-change
+  # Rebase your change onto the latest commits on Github.
+  % git pull --rebase origin main
----------------
I'm a big fan of:

```
$ git rebase --onto main --root branch-with-change
```
which will rebase branch-with-change onto main, then checkout branch-with-change.


================
Comment at: llvm/docs/Contributing.rst:144
+  # Push to Github.
+  % git push origin HEAD:main
+
----------------
Isn't it necessary to merge branch-with-change into main before pushing main?

i.e. I would do:

```
$ git checkout main
$ git pull
$ git rebase --onto main --root branch-with-change
$ ninja check-$whatever
$ git checkout main
$ git merge -
$ git pull --rebase
$ git push
```
I guess you're suggesting pushing from the development branch? I guess that should be fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129255