-
Notifications
You must be signed in to change notification settings - Fork 12.6k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Update llvm/docs/MyFirstTypoFix.rst for post-Phabricator / Pull-reque…
…sts world (#70310)
- Loading branch information
Showing
1 changed file
with
15 additions
and
84 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
|
@@ -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 | ||
~~~~~~~~ | ||
|
||
|
@@ -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 | ||
------------------- | ||
|
@@ -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 | ||
------------------ | ||
|
||
|
@@ -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 | ||
|