Skip to content

Update llvm/docs/MyFirstTypoFix.rst for post-Phabricator / Pull-requests world #70310

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 26, 2023
Merged

Update llvm/docs/MyFirstTypoFix.rst for post-Phabricator / Pull-requests world #70310

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 15 additions & 84 deletions llvm/docs/MyFirstTypoFix.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,6 @@ We're going to need some tools:

- python: to run the LLVM tests,

- arcanist: for uploading changes for review,

As an example, on Ubuntu:

.. code:: console
Expand Down Expand Up @@ -345,42 +343,21 @@ all the \*-commits mailing lists).
Uploading a change for review
-----------------------------

.. warning::

Phabricator is deprecated and will be switched to read-only mode in October
2023. For new code contributions use :ref:`GitHub Pull Requests <github-reviews>`.
This section contains old information that needs to be updated.

LLVM code reviews happen at https://reviews.llvm.org. The web interface
is called Phabricator, and the code review part is Differential. You
should create a user account there for reviews (click "Log In" and then
"Register new account").

Now you can upload your change for review:

.. code:: console

$ arc diff HEAD^

This creates a review for your change, comparing your current commit
with the previous commit. You will be prompted to fill in the review
details. Your commit message is already there, so just add cfe-commits
under the "subscribers" section. It should print a code review URL:
https://reviews.llvm.org/D58291 You can always find your active reviews
on Phabricator under "My activity".

LLVM code reviews happen through pull-request on GitHub, see
:ref:`GitHub <github-reviews>` documentation for how to open
a pull-request on GitHub.

Review process
--------------

When you upload a change for review, an email is sent to you, the
cfe-commits list, and anyone else subscribed to these kinds of changes.
When you open a pull-request, some automation will add a comment and
notify different member of the projects depending on the component you
changed.
Within a few days, someone should start the review. They may add
themselves as a reviewer, or simply start leaving comments. You'll get
another email any time the review is updated. The details are in the
`https://llvm.org/docs/CodeReview/ <https://llvm.org/docs/CodeReview.html>`__.


Comments
~~~~~~~~

Expand All @@ -395,37 +372,30 @@ page.
Updating your change
~~~~~~~~~~~~~~~~~~~~

If you make changes in response to a reviewer's comments, simply run

.. code:: console

$ arc diff

again to update the change and notify the reviewer. Typically this is a
good time to send any draft comments as well.

If you make changes in response to a reviewer's comments, simply update
your branch with more commits and push to your fork. It may be a good
idea to answer the comments from the reviewer explicitly.

Accepting a revision
~~~~~~~~~~~~~~~~~~~~

When the reviewer is happy with the change, they will **Accept** the
revision. They may leave some more minor comments that you should
address, but at this point the review is complete. It's time to get it
committed!
merged!


Commit by proxy
---------------

As this is your first change, you won't have access to commit it
As this is your first change, you won't have access to merge it
yourself yet. The reviewer **doesn't know this**, so you need to tell
them! Leave a message on the review like:

Thanks @somellvmdev. I don't have commit access, can you land this
patch for me? Please use "My Name my@email" to commit the change.

The review will be updated when the change is committed.
patch for me?

The pull-request will be closed and you will be notified by GitHub.

Review expectations
-------------------
Expand Down Expand Up @@ -484,46 +454,6 @@ changes (e.g. [email protected]), as discussion often happens
there if a new patch causes problems.


Commit
------

Let's say you have a change on a local git branch, reviewed and ready to
commit. Things to do first:

- if you used multiple fine-grained commits locally, squash them into a
single commit. LLVM prefers commits to match the code that was
reviewed. (If you created one commit and then used "arc diff", you're
fine)

- rebase your patch against the latest LLVM code. LLVM uses a linear
history, so everything should be based on an up-to-date origin/main.

.. code:: console

$ git pull --rebase https://github.com/llvm/llvm-project.git main

- ensure the patch looks correct.

.. code:: console

$ git show

- run the tests one last time, for good luck

At this point git show should show a single commit on top of
origin/main.

Now you can push your commit with

.. code:: console

$ git push https://github.com/llvm/llvm-project.git HEAD:main

You should see your change `on
GitHub <https://github.com/llvm/llvm-project/commits/main>`__ within
minutes.


Post-commit errors
------------------

Expand Down Expand Up @@ -559,7 +489,8 @@ buildbots, overview of bots, getting error logs.
Reverts
-------

if in doubt, revert and re-land.
If in doubt, revert immediately, and re-land later after investigation
and fix.


Conclusion
Expand Down