You are viewing an old version of this page. View the current version.

Compare with Current View Page History

« Previous Version 5 Next »

This page explains how to use Phabricator for code review of contributions to Apache Hive.  Phabricator usage is currently under test; the official code review tool is still Review Board, but we're thinking about changing that, so please help test out Phabricator and let us know what you think on the Hive mailing lists.

Setup

  1. Visit https://reviews.facebook.net and connect your existing github or Facebook account there.
  2. The Phabricator command-line tool arc requires a PHP interpreter available on your development machine.  If you can run the command php --version, you already have it.  If not, google for "php command line" to see how to install it for your system.
  3. Next, install arc.
  4. In your Hive checkout (either git or svn), run arc install-certificate and follow the instructions given.
  5. Run ant arc-setup to install an extra module needed for JIRA integration.  (This ant target was added with the commit of HIVE-2486.)

That's it, you're ready to use Phabricator!

Submitting Patches (git)

From a git checkout (e.g. from git clone git://github.com/apache/hive.git), a typical usage sequence is as follows:

  1. ant arc-setup
  2. git checkout -b HIVE-123-dev-branch
  3. edit some files to implement HIVE-123
  4. git commit -a -m "initial description of how I have gone about making DDL code suck less"
  5. arc diff --jira HIVE-123
  6. optionally, visit diff URL provided by Phabricator to do things like add reviewers and CC's

Note that at this point, your diff has already been submitted as a patch against JIRA, along with a corresponding comment in JIRA containing a link back to the new diff.

If you just want to see what your diff looks like, without actually creating it, use --preview to the arc diff command line (but don't proceed with creating the diff when you visit it in the UI since it will be missing the JIRA information).

At this point, reviewers can add comments and request changes.  To address them:

  1. git checkout HIVE-123-dev-branch
  2. edit some more files
  3. git commit -a --amend -C HEAD (this will keep the existing log message, which is important to Phabricator)
  4. arc diff (you'll be required to provide a description of your changes here; this will be shown to reviewers in the UI)

Repeat until all reviewers are satisfied.  Note that you only need to specify --jira on the very first call to arc diff; after that, arc will get it from the revision ID stored in the git log.

Submitting Patches (svn)

Work in progress...best to use git for now, or keep using Review Board + svn.

Reviewing Patches

When using the Differential UI to review a diff:

  • Click on a line number in code to add a line-specific comment
  • You can also add general comments in the box at the bottom
  • When done, be sure to choose an action (e.g. Request Changes) from the dropdown at the bottom, and then click the Clowncopterize button.  Without this, the contributor+JIRA won't see your changes.

Committing Patches (svn)

If you are a committer, you can apply a patch from revision D123 to be committed with:

  1. svn update
  2. ant arc-setup
  3. arc patch --revision 123

Amazingly, it will actually svn add/remove files for you automatically, so you don't have to (as you would with a raw patch command).  Wow!  Progress!

For now, continue to use svn commit.  In the future we'll be using arc commit instead so that Phabricator will be able to automatically detect that the revision has been committed.  You can do this manually by following the svn commit with arc mark-committed --revision 123.

Limitations

  1. svn contributor support
  2. arc commit log format that will satisfy JIRA, Jenkins and Phabricator (without this, contributors need to manually mark their revisions as committed)
  3. there's currently no way for contributors to grant the ASF rights on their patch except to go to JIRA and re-add the last version of the patch there, clicking the correct radio button; we haven't thought of an acceptable solution to this, so please let us know if you have any bright ideas
  4. it would be nice if submitting a diff and requesting changes automatically updated the JIRA "Patch Available" status
  5. it would be nice to keep using the old patch naming convention, e.g. HIVE-123.1.patch instead of D23.1.patch
  6. lint, unit tests, etc
  • No labels