Manuel Klimek | 81eb88f | 2012-10-11 19:40:46 +0000 | [diff] [blame] | 1 | ============================= |
| 2 | Code Reviews with Phabricator |
| 3 | ============================= |
| 4 | |
| 5 | .. contents:: |
| 6 | :local: |
| 7 | |
| 8 | If you prefer to use a web user interface for code reviews, |
| 9 | you can now submit your patches for Clang and LLVM at |
| 10 | `LLVM's Phabricator`_. |
| 11 | |
| 12 | Sign up |
| 13 | ------- |
| 14 | |
Chandler Carruth | d62fd65 | 2012-10-27 09:47:33 +0000 | [diff] [blame] | 15 | There are two options to get an account on Phabricator. You can sign up |
| 16 | immediately with one of the supported OAuth account types if you're comfortable |
| 17 | with OAuth, but you can also email chandlerc@gmail.com to request an account to |
| 18 | be created manually without using OAuth. We're working to get support in |
| 19 | Phabricator to directly create new accounts, but currently this is a manual |
| 20 | process. |
| 21 | |
| 22 | Note that if you use your Subversion user name as Phabricator user name, |
| 23 | Phabricator will automatically connect your submits to your Phabricator user in |
| 24 | the `Code Repository Browser`_. |
Manuel Klimek | 81eb88f | 2012-10-11 19:40:46 +0000 | [diff] [blame] | 25 | |
| 26 | |
| 27 | Requesting a review via the command line |
| 28 | ---------------------------------------- |
| 29 | |
| 30 | Phabricator has a tool called *Arcanist* to upload patches from |
| 31 | the command line. To get you set up, follow the |
| 32 | `Arcanist Quick Start`_ instructions. |
| 33 | |
| 34 | You can learn more about how to use arc to interact with |
| 35 | Phabricator in the `Arcanist User Guide`_. |
| 36 | |
| 37 | Requesting a review via the web interface |
| 38 | ----------------------------------------- |
| 39 | |
| 40 | The tool to create and review patches in Phabricator is called |
| 41 | *Differential*. |
| 42 | |
| 43 | Note that you can upload patches created through various diff tools, |
| 44 | including git and svn. To make reviews easier, please always include |
| 45 | **as much context as possible** with your diff! Don't worry, Phabricator |
| 46 | will automatically send a diff with a smaller context in the review |
| 47 | email, but having the full file in the web interface will help the |
| 48 | reviewer understand your code. |
| 49 | |
| 50 | To get a full diff, use one of the following commands (or just use Arcanist |
| 51 | to upload your patch): |
| 52 | |
Alexey Samsonov | 4db4a71 | 2012-11-06 15:04:37 +0000 | [diff] [blame] | 53 | * ``git diff -U999999 other-branch`` |
| 54 | * ``svn diff --diff-cmd=diff -x -U999999`` |
Manuel Klimek | 81eb88f | 2012-10-11 19:40:46 +0000 | [diff] [blame] | 55 | |
| 56 | To upload a new patch: |
| 57 | |
| 58 | * Click *Differential*. |
| 59 | * Click *Create Revision*. |
| 60 | * Paste the text diff or upload the patch file. |
| 61 | Note that TODO |
| 62 | * Leave the drop down on *Create a new Revision...* and click *Continue*. |
| 63 | * Enter a descriptive title and summary; add reviewers and mailing |
| 64 | lists that you want to be included in the review. If your patch is |
| 65 | for LLVM, cc llvm-commits; if your patch is for Clang, cc cfe-commits. |
| 66 | * Click *Save*. |
| 67 | |
| 68 | To submit an updated patch: |
| 69 | |
| 70 | * Click *Differential*. |
| 71 | * Click *Create Revision*. |
| 72 | * Paste the updated diff. |
| 73 | * Select the review you want to from the *Attach To* dropdown and click |
| 74 | *Continue*. |
| 75 | * Click *Save*. |
| 76 | |
| 77 | Reviewing code with Phabricator |
| 78 | ------------------------------- |
| 79 | |
| 80 | Phabricator allows you to add inline comments as well as overall comments |
| 81 | to a revision. To add an inline comment, select the lines of code you want |
| 82 | to comment on by clicking and dragging the line numbers in the diff pane. |
| 83 | |
| 84 | You can add overall comments or submit your comments at the bottom of the page. |
| 85 | |
| 86 | Phabricator has many useful features, for example allowing you to select |
| 87 | diffs between different versions of the patch as it was reviewed in the |
| 88 | *Revision Update History*. Most features are self descriptive - explore, and |
| 89 | if you have a question, drop by on #llvm in IRC to get help. |
| 90 | |
| 91 | Status |
| 92 | ------ |
| 93 | |
| 94 | Currently, we're testing Phabricator for use with Clang/LLVM. Please let us |
| 95 | know whether you like it and what could be improved! |
Sean Silva | b92dfe0 | 2012-10-12 01:21:24 +0000 | [diff] [blame] | 96 | |
| 97 | .. _LLVM's Phabricator: http://llvm-reviews.chandlerc.com |
| 98 | .. _Code Repository Browser: http://llvm-reviews.chandlerc.com/diffusion/ |
| 99 | .. _Arcanist Quick Start: http://www.phabricator.com/docs/phabricator/article/Arcanist_Quick_Start.html |
| 100 | .. _Arcanist User Guide: http://www.phabricator.com/docs/phabricator/article/Arcanist_User_Guide.html |