What is Phabricator
Phabricator is a software development collaboration tool and includes features such as code review and repository browsing. Facebook runs a Phabricator instance at reviews.facebook.net, which is where all pull requests on GitHub get sent to be reviewed by the HHVM team.
When you submit a pull request to HHVM, you will notice a comment from facebook-github-bot appear shortly after, with a link to a newly-created Phabricator review:
You do not have to register for Phabricator, although in order to discuss changes this allows you to directly converse with the change reviewers. To register, simply click the "standby" icon top right of Phabricator, and register using your GitHub account - this allows all future changes from your account to be created in your name (rather than facebook-github-bot). This means that when you go to reviews.facebook.net/differential/query/authored/, you will see all your changes in one place.
You might notice the following message on your review:
If your change is fully covered by new/changed unit tests, you don't need to worry about this field.
If your change is not covered by unit tests, this field should:
- explain why it cannot be covered by automated tests
- give enough information for someone unfamiliar with the subsystem to test the change fully, and understand what they're testing
If all the reviewers for a change are happy your revision will become "Accepted":
Once that happens, the @hhvm-bot team will automatically land the change into GitHub. You should not click the "Land to GitHub" button.
Need to write this section...
The review team will add any appropriate reviewers to the change. Unless you know specifically someone who should be reviewing the code, it is safe to leave this to the HHVM team.
Need to write this section...
If you keep seeing additions to the review such as this:
but you are sure you did not change anything, this is probably normal, and likely due to the master branch updating so Phabricator will re-base the changes on the latest revisions. It is safe to ignore these (unless you did actually create a new diff). An example of this happening can also be seen here.