In a previous post (Part 1) we introduced how the Atlassian Bamboo developers were using Crucible for their code review process. We showed a snapshot of why the Bamboo developers do code review and how they get the process started.
In this post you will learn about how the Bamboo developers perform code reviews and some tips and tricks they have learned along the way.
Before we jump into the post, let’s review the number of ways (and places) to create Crucible code reviews.
Crucible web interface – from the Crucible interface you can create code reviews with a workflow or share you code without the need for a formal code review workflow with Snippet Reviews or Changeset discussions.
FishEye – if you are running Atlassians source code insight tool, FishEye, alongside Crucible you can create a review from any revision, diff or changeset that FishEye displays.
JIRA – if you have FishEye, you can create code reviews from one or all commits related to an issue in directly from your JIRA issue tracker.
IDE – using the Atlassian IDE Connectors for Eclipse or IntelliJ IDEA, you can create pre-commit reviews from your workspace or automatically create reviews post commit.
Bamboo developers code review process continued
Below is a continuation of the snapshot of the process the developers of Bamboo use to document the code review process (Part 1) including tips they have learned along the way.
Performing a code review
Like a lot of things in life, code reviews are about balance. You need to put the value of painstakingly and critically evaluating every line of code, line by line. Remember the whys of code review, you don’t have to find every single possible bug or optimize each line of code, it’s about making sure you get what’s going on in the source and hopefully improving the quality of what gets released along the way. It’s the responsibility of the developer (and the tech lead) to make sure code is of acceptable quality. Scanning through files and reading commented items is better than not doing anything at all. Try not to slack but don’t burn yourself out either.
Responding to code reviews
Once you’ve created a code review, you still need to follow up on it.
- You should respond to comments as soon as you can. There is no need to wait for other reviewers who are slow at code reviews.
- All comments should be responded to. At the minimum, acknowledge a comment with a response of “Done” so we know the comment has been looked at.
Tips and FAQs
What should I create a review for?
You should create a code review for all changes relating to an issue you’re working on. Trivial reviews takes a trivial amount of time to do, so it’s not really that hard.
How do I create a code review?
You can create reviews from Crucible, FishEye, JIRA or your IDE. From FishEye, use an RSS feed to identify the commit and use the Tools menu to quickly create a review for that commit. This is useful for partial reviews.
If you are creating a review from JIRA go through JIRA’s issue tab panels. On an issue, click “All” or “Source” and you should see a link to the Crucible project(s). From there you can simply create a review.
You can create code reviews through Crucible, but I find FishEye and JIRA easier.
Who gets added to reviews?
By default everyone from the Bamboo team (a group in FishEye and Crucible) is added to the review. Other users can be added if there is a need – an example of this is adding a member from the UI team to review any changes to the GUI.
What if I just don’t understand something?
Then you definitely ask! We should aim to write code that’s maintainable by anyone on the team. If you have no idea what going on with part of the source code then we need to fix that problem.
Iterative reviews or new reviews?
It’s a personal taste thing. But if the original review is big or getting out of date then I think it’s better to just create a new review.
How should I do massive code reviews?
Scan and conquer – there are usually more important and less important parts to a review, so try to focus on the least important files first. For the Bamboo team we have auto mark file turned off in Crucible, and quickly check off small changes. This usually trims the review down to a more manageable level.