Code Reviews

The OpenClonk project is using Atlassian Crucible to conduct code reviews. Code reviews help to improve the quality of our source code by having more than one set of eyes look at it.

Request a Code Review

To start a code review, either as a reviewer or a reviewee, log into Crucible and click the Create Review button in your toolbar. You will be asked to select a project for the review; use the one that most closely fits the patch you want to review. If no project fits well, you can use the OpenClonk project (OC) as a fall-back choice. Afterwards, click the Create Review button.

Once you have created the review in a specific project, you will have to add content to the review. If your code has not yet been pushed, you will have to generate a patch file and upload that. If the code is already contained in the master repository, you can select the changeset directly.

Review an un-pushed patch

To review a patch that has not yet been pushed to the master repository, use the Pre-commit link. You will be asked to upload or paste a patch file. How to generate this file differs depending on the Git UI you're using; for the command line UI, it works as follows:

If you have staged, but not committed your changes:

$ git diff --cached > some-file-name.patch

If you have already committed your changes:

$ git diff HEAD~ > some-file-name.patch

This will emit a patch into the file some-file-name.patch. This is the file you will want to upload to or paste into Crucible.

Note: Do not use the git format-patch command, as it will generate a file in mbox format instead of a plain patch.

Note: Crucible also offers a python script that promises easier review creation. Unfortunately it currently suffers from a bug that occasionally introduces a spurious whitespace change. As such we recommend you do not use it.

Once uploaded, Crucible will try to anchor your patch to the OpenClonk repository. This will enable people reviewing the patch to see the complete source code of the files instead of just the changes. If this fails, it may mean that the patch does not cleanly apply to the most recent changeset on the master branch.

If the patch successfully anchored to the repository, or you want to proceed regardless, click the Finish upload button.

Review a pushed changeset

To review a changeset that has already been pushed to the master repository, use the Browse Changesets link. You will be shown a list of your most recent changesets. Check the revision (or revisions) that you want to have reviewed.

Give the review a title

Once you added the content to review, you should give the review a title so people know what to expect. To do this, click the Edit Details button. If you added a changeset, the title will already be filled in with the title of the changeset. Otherwise, you'll have to enter one yourself. If you added a changeset, you can also change the author role of the review to the author of the changeset, so they know their code is being reviewed.

Then, add some reviewers. If you are trying to review somebody else's code, this should include yourself. If you don't know who should review the code, you can have Crucible suggest somebody; you can also hop in the IRC channel and ask if somebody wants to do the review, or just leave it empty so people can join later. Make sure you leave the Allow anyone to join checkbox checked.

Once that's all done, you can stop drafting the review and start it. To do that, click the Start Review button. If you did not specify any reviewers, you will get a warning message telling you so; confirm that you meant to do this and start handing out the review link. If you did specify reviewers, the people you chose will get an e-mail notification.

Mail settings

By default, Crucible will send members of a review an e-mail notification about pretty much everything that happens. If you do not want to get your mailbox filled up, you should change your notification settings.

To do that, click your profile picture in the top right corner of the screen and choose Profile settings. In there, go to the Review settings and switch some or all of the events to No or Batch. Batch will collect changes for about 30 minutes and then send an e-mail containing all changes that happened in the meantime.