Dev:Doc/Tools/Code Review

Code Review

Differential on developer.blender.org is an online tool for code review, used by developers with commit access to review each others code, and by other developers to submit patches.

Doing code review improves the quality of commits. With more eyes looking at the code we can prevent bugs, and having your code reviewed also generally keeps you on your toes. If mistakes slip through anyway, Audit on developer.blender.org can be used to notify developers and review code after it has been committed.

Guidelines

The intention is that code review is used for most non-trivial commits. Some guidelines:

• The typical reviewers with final approval should be module owners, but anyone can and is encouraged to participate.
• Code reviewers should try to either accept, request changes, or reassign to another reviewer quickly, to unblock the developers working on their features. This makes the process more inviting to new developers and more usable for existing developers.
• Developers submitting code should make it easy for people to review their code. That means you do not submit huge patches, but break up your work into multiple smaller commits and reviews. Code refactoring, new features, bug fixes, etc. should generally be submitted as separate code reviews. You can have them as separate commits in the same local Git branch.
• If there are bigger design question to solve, open a Patch or Design task first, so that code review can focus mostly on the implementation.

Use a Local Branch

When you are going to submit code for review, it is best to commit that code in a local Git branch.

git checkout -b some-new-feature
# ... make changes to code ...
git commit .

The next step is to create a revision in Differential, and there are two different ways of doing this.

Upload a Diff

The easiest way to create a revision in Differential is to Create a Diff and upload a diff file there. For detailed information on how to work with such files, see the Patches and Diffs page.

In short, Differential expects standard git diff or git show output. If your changes are committed in a local branch, this is the command that will work.

# generate a diff from some-new-feature branch
git diff master...some-new-feature > some-new-feature.diff

When the reviewer requests changes, you can use Update Diff on the code review page to upload a new patch.

Use Arcanist

Phabricator comes with a command line tool called arc which can be used to quickly submit, apply, and land revisions. For developers that do code reviews often this is much more efficient than uploading and downloading diffs, but requires some setup and use of command line tools.

Installation

See the Arcanist Quick Start.

Basically you need to install php, get arcanist from its git repository, and put the arc command in your PATH.

You do not need to add a .arcconfig, this is already in the repository.

Next you need to authenticate yourself. Run this command and follow the instructions:

arc install-certificate

Send a Patch

When arc is installed and all your code changes are committed in a branch, sending the changes for review is quick, simply run:

git checkout some-new-feature
arc diff

You will be prompted to add a title on the first line, and an optional extra summary and reviewers. If this revision relates to a particular task, you should mention the task ID in the summary, for example "This commit fixes T123" to reference task T123. The revision will then shown up automatically in the task. The reviewers are referred to by their username on developer.blender.org.

To update the revision with modified or new commits, simply run the same arc diff command again. If updating a revision you may want to get the latest changes from master on your local branch (should not rebase on public branch) before submitting a new diff, this can be done with:

# commit code to your branch if not already done so
git checkout master
git pull origin master
git checkout some-new-feature
git rebase master

Apply a Patch

A reviewer or other interested party can download and apply a patch to their local repository with a command like this, which will be shown on the revision page:

arc patch D123

This will download the patch and apply it in a newly created branch.

Land a Patch

When the revision is approved and you have commit rights, you can push to the main repository. If you do not have commit rights, the reviewer will do this for you.

It's best to use this command to move it to the master branch:

arc land --hold some-new-feature

Then you can inspect the commit message and if necessary amend it:

# show commit log and diff
git show HEAD
# change commit log
git commit --amend

Some Other Useful Commands

This section explained the basic workflow for working with arc, however it is quite a flexible tool and it can be used in different ways. For example you can send a specified range of commits, or even uncommitted changes rather than always creating a branch, it's a matter of preference.

arc can also be used to close revisions, close tasks, upload or download files, and many more things. See the user guide for more details. Here are a few useful commands:

# list local branches that have an associated revision on differential
arc feature
# list open tasks assigned to you
arc tasks
# upgrade to the latest version of arc (can be needed when we update the website)
arc upgrade

Further Reading

• User Guide
• arc diff Guide