|The information below assumes you are familiar with Git. If not, we recommend to visit the Check-Out & First Steps section first.
In the Saros project we perform a peer review of all patches before they are submitted to the central repository. Learn more about whys and hows of our review process
For larger development efforts (e.g. a new feature, large-scale re-engineering), you can request a branch in which to work. The problem this introduces is that when it comes to review the changes, the resulting patch is far too big. However, we can take advantage of ReviewBoard?
features to manage this.
Therefore, granting a branch for your work will only be agreed if you define a set of milestones at which you intend to release new code for review and get it approved. Each milestone should be a meaningful "checkpoint" that helps to show the reviewers where you're heading and get approval for it. You do this by building up a patch in stages and so the reviewers only have to look at the changes since the last approved patch version. We use ReviewBoard?
to manage it for us and we'll call it an "ongoing review request" (ORR).
At your first checkpoint, open a review request on ReviewBoard?
with the first version of your patch
Wait for approval (a pair of +1s)
If you update the patch while awaiting approval, you lose any +1s you've already received.
If you finally get approval, record the patch version number (from ReviewBoard?
) for later.
You do not have to commit your changes to the trunk.
Continue working on your branch.
When you reach your next milestone, make another patch and update the ORR
Important! When you announce your review request, you must include the patch version number recorded earlier, e.g. r2. Reviewers can then use ReviewBoard?
to show only the differences between the current patch version and r2.
Go back to step 2.
This section contains a couple of guidelines for renaming classes or methods. They are worth sticking to because of our pre-commit review process, which requires you to create patches, which can be problematic in combination with renaming.
Rule Nr. 1: Beware of renaming classes. If you want to rename a class, put the Refactor->Rename in a separate patch, and post/commit it as soon as possible.
If you rename a class and make additional changes to that class, your patch becomes hard to review, because the additional changes are now in a different file. The Compare editor in Eclipse and the Diff view in the Reviewboard won't be able to display what the additional change was.
Even worse, what if someone else makes changes to that class? You renamed it locally, and someone else commited a change to the "old" class, how are you going to merge that? You'll be in integration hell faster than you can say "Refactoring is easy in Java".