A common question that comes up is how do the Atlassians use their own tools. Whether you’re on the Confluence, Jira, Fisheye, Crucible, Bamboo, Crowd, Plugin development, or any other team you use the Atlassian toolset as part of the development process.
One point to remember is that different teams may use the tools differently. This two part blog series will focus on how the Atlassian Bamboo developers use Crucible for their code review process.
Before we jump into the series lets quickly review several different ways Crucible can assist in the code review process.
- Pre-commit: reviewing content by uploading a file, patch or directly from IDE.
- Post-commit: reviewing code that is committed to the repository.
- Snippet: cut and paste code or any text, without the need for a formal code review workflow.
- Changeset discussions: provide free form comments on commit messages.
There are many different ways to get started with code review. Check out the Code Review for Busy Teams presentation from Atlassian Summit to get ideas on how to incorporate code review into your daily development practices.
Bamboo developers code review process
Below is a snapshot of the process the developers of Bamboo use to document the code review process. This is a constantly evolving document and changes are always made to improve efficiency.
Why we code review?
- Everyone is aware of what code is being checked. This is an opportunity for the team to be updated on everything that is going on with the source code.
- Learn from other developers code.
- Know what’s happening around the Bamboo source code.
- Reduce WTF moments – maintaining code shouldn’t be the first time you’ve laid eyes on something.
- Make sure that the code works.
The list is in order of importance. This might seem strange. Why is “Make sure the code works” all the way down at the bottom? Isn’t picking up bugs the point of the code review? Not exactly. It is not the responsibility of others to pick up on your mistakes. Having everyone review your code is not an excuse for getting sloppy with your own code. That is the main reason it is at the top.
Why does everyone review everything?
Everyone brings their own eyes to a problem. Some are pixel pushers, some are test zealots, some are concurrency nerds, some are standards freaks, etc. Everyone brings their own perspective in what makes for better code.
Creating a code review
There’s going to be a lot of reviews (hopefully). What this means is for each line you submit, for each file you submit, there’s going to have to be another 6 sets of eyes that will have to look through that source code.
If there is something that you can do to reduce the time it takes to do that review by 5 minutes, you’re already saving half an hour’s worth of work for everyone. So basically, spend time upfront creating good code reviews and your mates will thank you for it.
- Break it up – small code reviews are more easily digestible than a “monster” review.
- Once over your code (pre-commit) – try to make it a habit to look over the source you are submitting for code review.
- Cohesion counts – like code, you want a single Code Review to be logically together. Refactor name changes of massively used classes, commits, and then create a new review with your actual code changes – isolate to the core of the review.
- Comment on the review – the methods + javadoc + tests should make the code self documenting. Code (and tests) should tell you what. Code doesn’t tell you why the change was made, however, it deals with the final product. If the reason is possibly unclear, add a Crucible comment. If you’re unsure of something, add a Crucible comment. If there is something you think people need to pay attention to, add a Crucible comment.
What’s the best way to creating better code reviews? Strangely, it’s not creating more code reviews, it’s doing them. Doing lots of code reviews means you see what really pisses you off about doing them and what bits and pieces are really useful. You don’t need to spend hours prepping your code reviews, just spend a bit of time looking through them. Breaking up a massive commit is not easy, but then I never said it was easy. It’s just better when one person deals with it rather than having 6 people deal with it.
More code review with the Bamboo developers
In the second and final part of the blog series we will learn about how the Bamboo team responds to code reviews and some tips they have learned along the way.