Skip to content

Commit

Permalink
[docs] Move code contribution from GettingStarted.rst to Contributing…
Browse files Browse the repository at this point in the history 
….rst

For code contribution, GettingStarted.rst duplicates information in Contributing.rst.
The dedicated Contributing.rst is a better place for code contribution, so move
the content there.

Notes:

* D41665 added `Contributing.rst`
* D110976 mentioned `git cherry-pick e3659d43d8911e91739f3b0c5935598bceb859aa` workaround

Reviewed By: cjdb, fhahn, nickdesaulniers

Differential Revision: https://reviews.llvm.org/D129255
  • Loading branch information
@MaskRay
MaskRay committed Jul 7, 2022
1 parent ff8c0e6 commit 472aa7e
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 108 deletions.
58 changes: 53 additions & 5 deletions llvm/docs/Contributing.rst
View file
Expand Up @@ -48,6 +48,8 @@ interesting projects is maintained at the `LLVM's Open Projects page`_. In case
you are interested in working on any of these projects, please post on the
`Forum`_, so that we know the project is being worked on.

.. _submit_patch:

How to Submit a Patch
=====================
Once you have a patch ready, it is time to submit it. The patch should:
Expand All @@ -56,6 +58,7 @@ Once you have a patch ready, it is time to submit it. The patch should:
* 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.
* have a single commit (unless stacked on another Differential), up-to-date with the upstream ``origin/main`` branch, and don't have merges.

.. _format patches:

Expand Down Expand Up @@ -85,11 +88,10 @@ in order to update the last commit with all pending changes.
the git integration can be run from
``clang/tools/clang-format/git-clang-format``.


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>`.
(We used to allow patches on the llvm-commits mailing list, but the mailing lists
have been deprecated.)

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
Expand All @@ -115,6 +117,52 @@ professional developers.

For more information on LLVM's code-review process, please see :doc:`CodeReview`.

.. _commit_from_git:

For developers to commit changes from Git
-----------------------------------------

Once a patch is reviewed, you should rebase it, re-test locally, and commit the
changes to LLVM's main branch. This is done using `git push` if you have the
required access rights. See `committing a change
<Phabricator.html#committing-a-change>`_ for Phabricator based commits or
`obtaining commit access <DeveloperPolicy.html#obtaining-commit-access>`_
for commit access.

Here is an example workflow using git. This workflow assumes you have an
accepted commit on the branch named `branch-with-change`.

.. code-block:: console
# Pull changes from the upstream main branch.
% git checkout main && git pull
# Rebase your change onto main.
% git rebase --onto main --root branch-with-change
# Rerun the appropriate tests if needed.
% ninja check-$whatever
# Check that the list of commits about to be pushed is correct.
% git log origin/main...HEAD --oneline
# Push to Github.
% git push origin HEAD:main
LLVM currently has a linear-history policy, which means that merge commits are
not allowed. The `llvm-project` repo on github is configured to reject pushes
that include merges, so the `git rebase` step above is required.

Please ask for help if you're having trouble with your particular git workflow.

.. _git_pre_push_hook:

Git pre-push hook
^^^^^^^^^^^^^^^^^

We include an optional pre-push hook that run some sanity checks on the revisions
you are about to push and ask confirmation if you push multiple commits at once.
You can set it up (on Unix systems) by running from the repository root:

.. code-block:: console
% ln -sf ../../llvm/utils/git/pre-push.py .git/hooks/pre-push
Helpful Information About LLVM
==============================
Expand Down
104 changes: 1 addition & 103 deletions llvm/docs/GettingStarted.rst
View file
Expand Up @@ -458,107 +458,7 @@ command. Use `git tag -l` to list all of them.
Sending patches
^^^^^^^^^^^^^^^

Please read `Developer Policy <DeveloperPolicy.html#one-off-patches>`_, too.

We don't currently accept github pull requests, so you'll need to send patches
either via emailing to llvm-commits, or, preferably, via :ref:`Phabricator
<phabricator-reviews>`.

You'll generally want to make sure your branch has a single commit,
corresponding to the review you wish to send, up-to-date with the upstream
``origin/main`` branch, and doesn't contain merges. Once you have that, you
can start `a Phabricator review <Phabricator.html>`_ (or use ``git show`` or
``git format-patch`` to output the diff, and attach it to an email message).

However, using the "Arcanist" tool is often easier. After `installing arcanist`_, you
will also need to apply a fix to your arcanist repo in order to submit a patch:

.. code-block:: console
% cd arcanist
% git fetch https://github.com/rashkov/arcanist update_cacerts
% git cherry-pick e3659d43d8911e91739f3b0c5935598bceb859aa
Once this is all done, you can upload the latest commit using:

.. code-block:: console
% arc diff HEAD~1
Additionally, before sending a patch for review, please also try to ensure it's
formatted properly. We use ``clang-format`` for this, which has git integration
through the ``git-clang-format`` script. On some systems, it may already be
installed (or be installable via your package manager). If so, you can simply
run it -- the following command will format only the code changed in the most
recent commit:

.. code-block:: console
% git clang-format HEAD~1
Note that this modifies the files, but doesn't commit them -- you'll likely want
to run

.. code-block:: console
% git commit --amend -a
in order to update the last commit with all pending changes.

.. note::
If you don't already have ``clang-format`` or ``git clang-format`` installed
on your system, the ``clang-format`` binary will be built alongside clang, and
the git integration can be run from
``clang/tools/clang-format/git-clang-format``.


.. _commit_from_git:

For developers to commit changes from Git
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Once a patch is reviewed, you should rebase it, re-test locally, and commit the
changes to LLVM's main branch. This is done using `git push` if you have the
required access rights. See `committing a change
<Phabricator.html#committing-a-change>`_ for Phabricator based commits or
`obtaining commit access <DeveloperPolicy.html#obtaining-commit-access>`_
for commit access.

Here is an example workflow using git. This workflow assumes you have an
accepted commit on the branch named `branch-with-change`.

.. code-block:: console
# Go to the branch with your accepted commit.
% git checkout branch-with-change
# Rebase your change onto the latest commits on Github.
% git pull --rebase origin main
# Rerun the appropriate tests if needed.
% ninja check-$whatever
# Check that the list of commits about to be pushed is correct.
% git log origin/main...HEAD --oneline
# Push to Github.
% git push origin HEAD:main
LLVM currently has a linear-history policy, which means that merge commits are
not allowed. The `llvm-project` repo on github is configured to reject pushes
that include merges, so the `git rebase` step above is required.

Please ask for help if you're having trouble with your particular git workflow.


.. _git_pre_push_hook:

Git pre-push hook
^^^^^^^^^^^^^^^^^

We include an optional pre-push hook that run some sanity checks on the revisions
you are about to push and ask confirmation if you push multiple commits at once.
You can set it up (on Unix systems) by running from the repository root:

.. code-block:: console
% ln -sf ../../llvm/utils/git/pre-push.py .git/hooks/pre-push
See :ref:`Contributing <submit_patch>`.

Bisecting commits
^^^^^^^^^^^^^^^^^
Expand Down Expand Up @@ -1248,5 +1148,3 @@ write something up!). For more information about LLVM, check out:
* `LLVM Homepage <https://llvm.org/>`_
* `LLVM Doxygen Tree <https://llvm.org/doxygen/>`_
* `Starting a Project that Uses LLVM <https://llvm.org/docs/Projects.html>`_

.. _installing arcanist: https://secure.phabricator.com/book/phabricator/article/arcanist_quick_start/
8 changes: 8 additions & 0 deletions llvm/docs/Phabricator.rst
View file
Expand Up @@ -37,6 +37,14 @@ Phabricator has a tool called *Arcanist* to upload patches from
the command line. To get you set up, follow the
`Arcanist Quick Start`_ instructions.

You may need to apply a fix to your arcanist repo in order to submit a patch:

.. code-block:: console
% cd arcanist
% git fetch https://github.com/rashkov/arcanist update_cacerts
% git cherry-pick e3659d43d8911e91739f3b0c5935598bceb859aa
You can learn more about how to use arc to interact with
Phabricator in the `Arcanist User Guide`_.
The basic way of creating a revision for the current commit in your local
Expand Down

0 comments on commit 472aa7e

Please sign in to comment.