Why your team should be doing code reviews

As developers, we strive to write great code and always ship our projects without bugs. Unfortunately, just like with anything else, there’s no such thing as ideal software. Issues do appear in our code all the time no matter how good we are or what sophisticated tools we use. So if we can’t entirely prevent bugs from happening, what can we do to protect our users from experiencing them?

Code review is a development process that was created specifically to address this problem. Instead of aiming at the impossible goal of writing code without errors, with code reviews you can instead focus on catching these errors before they make their way to your users. Code reviews bring on a colleague (or a few) to give you more eyes on your work. It simply doesn’t matter how many bugs your code had if you fixed all of them before your project got shipped! And it’s much easier and cheaper to fix them prior to the launch as well.

Having code reviews as part of your development workflow brings a lot of benefits to your entire team:

• Fewer bugs. Developers can continue making errors as usual, but code reviews will decrease the amount of those bugs that make it to production.
• Better security. Code reviews make it easy to spot potential vulnerabilities and fix them before they make their way to your servers.
• Knowledge sharing. Members of your team can learn from each other by reviewing each other’s code.
• Code quality. Things like readability, efficiency, and maintainability of your code may not always directly affect your users but they are tremendously important in the long run of your project.
• Better performance. Code reviews help spot performance issues and regressions.

This guide will help you integrate code reviews into your existing development workflow with no effort. The workflow was built around Beanstalk’s Code Review tools that make it super easy to conduct code reviews and ship better quality projects to your users. Now let’s jump right in!

Overview of the workflow

• Developers use branches to implement features and bug fixes.
• Once branches are ready for testing, developers request code reviews.
• Other members of your team review code from the branches.
• A list of issues is compiled for each review.
• Developers commit additional changes to the branches to fix discovered issues.
• Code review of the branch gets approved.
• Branch is merged into master and shipped to production.

Step 1: Write code and request a review

Create a branch and get to work

Work on every new feature or a bug fix always starts with the creation of a new branch. This will help you separate work that’s in progress from the stable version of your code and keep unfinished and experimental changes away from your production environment. Letting developers work in branches will also prevent them from stepping on each other’s toes: multiple features can be worked on in parallel without conflicting with each other. A solid code review process, as described in this guide, will help you verify branches before they get merged into the stable branch, further improving quality control.

Beanstalk makes it super easy to create new branches directly from the web app. It also keeps track of review and merge statuses for each branch automatically.

Request a code review when the branch is ready for testing

Right before a developer is ready to send their branch for testing, a code review should be requested. The reason why it makes sense to request a review before testing is to make sure that during the review process, developers are comfortable making additional changes to the code based on the feedback they are getting. We believe in starting a review early so you don’t get too far only to have to rewrite things after someone has made a great suggestion. If testing the branch has already begun, before a code review has started, then any changes that were introduced are going to invalidate the test results and the testing would have to start all over. This will make people reluctant to introduce changes during code review and will ultimately make them less useful.

The person who created the branch is usually the one who will be requesting a code review for that branch. If multiple developers are working on the same branch they should decide who is going to lead the effort and choose a person who will be requesting the review.

Often, reviews are assigned to more experienced team members to let the less experienced learn. At the same time, it’s important that all team members are participating in code reviews to promote knowledge sharing within your team. For example, a developer who never worked on a billing system in your application will be able to learn how it works if he or she is assigned to a review of another developer’s branch that updates the system.

Beanstalk allows assigning multiple people to each code review to increase the possibility of someone starting to work on it sooner and getting more opinions on the review. It makes sense to assign a review of a major feature to both a lead developer and a lead designer to have code implementation reviewed as well as the user interface.

More people can be added to code reviews as watchers to keep an eye on the progress or learn. Watchers will receive the same email digests with code review updates like assignees and the code review creator, but they won’t be able to approve the review. Team managers who like to keep track of the project’s progress would want to be included in code reviews as watchers.

How to prepare for a code review?

It’s important to make sure that the people who will be reviewing your branch have all the information they need to conduct the review:

• Make sure that your code is self-explanatory and has adequate amount of code documentation and comments.
• Include automated tests with your changes to make it easier to validate that your code works.
• If your branch is fixing a bug, include steps to reproduce the bug and any details that are necessary to verify the fix.
• When requesting a review, provide a short meaningful description of what your branch implements.

Step 2: Review code and submit feedback

Post comments on lines of code

Once a code review has been requested, each assignee will receive an email notification asking them to start their review process. The email will contain a quick summary of the branch to be reviewed. To start working on a review, the reviewer doesn’t have to click any buttons; a code review is considered to be in progress from the beginning until it’s closed.

The Code Review page in Beanstalk gives reviewers a great set of tools to conduct reviews quickly and effortlessly. The review process starts from the Code sub-page where a list of all files that were changed in the branch can be found together with their diffs. Reviewers can go through the code and comment right on the line of code to communicate with the branch author or other team members. These comments will appear both inside diff chunks on the Code sub-page and on the Discussion sub-page, together with the pieces of code they reference.

Discussions, however, are not limited to in-line comments. If you want to post a general comment about the branch or the review process it can be done directly on the Discussion sub-page.

Compile a list of issues

In addition to in-line comments, reviewers can also create issues. Issues are small actionable tasks that have to be completed before a code review can be approved. It’s the best way to pinpoint problems in code during a review. Code review participants can create issues from the Issues sub-page or directly from the Code sub-page just like in-line comments. When you comment on a line of code, you’ll have the option of making that comment an Issue, or leaving it a comment. Issues also support Markdown and @-syntax for mentioning users.

The biggest benefit of using issues to pinpoint problems in code instead of comments is that Beanstalk will keep track of issues that were already resolved and the ones that weren’t. You can find a list of all issues that were reported on the Issues sub-page and quickly get an idea of how much work is left.

Right now issues live only inside each code review, there’s no way to report them on a repository level. Once created they will stay inside a particular code review forever.

Tips for reviewing code

• Try not to review more than 400 lines of code at a time and keep your review sessions shorter than 90 minutes. It makes sense to split branches with a lot of code changes into multiple review sessions.
• Create a checklist of project-specific and programming language-specific things that you need to check during code reviews and use it for all reviews. The list can include things like checking if the code is documented appropriately, that it’s following your company’s code guidelines (or language’s general style guide), that features are covered with tests, that there is no code duplication, etc. It’s best to have that document available and used by the entire team.
• Make sure that the fix or feature that the branch implements actually works (duh!).
• During the review it’s better to submit many small comments and issues that are concise and actionable rather than a few very large comments that are hard to read and digest.

Tools that will help you conduct code reviews

Beanstalk provides a great set of tools to assist your team with the review process. I want to explain to you how some of these tools work, specifically Preview, Integrated Blame and Expand Changes.

Preview

Preview is a feature that is available for images and HTML files. It allows you to quickly preview your design mockups and see visual differences between the branch that’s being reviewed and the base branch. When being previewed, HTML files will be rendered with their CSS and JavaScript fully functional (with some limitations). This makes it possible to see how a mockup is going to behave in real life, including CSS animations, drop-down JS navigations and so forth. Preview is one of the reasons why it makes sense to include designers on code reviews so that they can verify visual correctness of the feature or bug fix.

Integrated Blame

Integrated blame is the commit information that appears when you click on a line of code to make a comment. It shows you the last person to change that line, when it happened, and what commit it was. You can then hover over the commit ID to see the commit message and click on it to see the complete commit. Integrated blame provides a meaningful context when you’re posting an in-line comment and helps you get the idea why that line was changed in the first place.

Expand changes

The “Expand changes” button expands any diff chunk on the Code sub-page into a list of commits related to this chunk of code. There are several cases when this feature can be extremely helpful. Have you ever wondered, while looking at a diff, why exactly this particular change was implemented? Or why the diff looks different from the last time you looked at it? Or perhaps who exactly from your team worked on it? Expand Changes will answer all of these questions by showing you exactly what commits a particular diff consists of, together with commit messages, dates and committer names.

Step 3: Fix discovered issues and finalize the review

Push updates to the branch and mark issues as resolved

Once reviewers start posting their first comments and issues, developers that are responsible for the branch can start working to fix them.

Every new commit made to the branch will show up on the History sub-page inside the code review. Every watcher (including assignees) will receive an email notification when this happens. Developers can then check off issues on the Issues sub-page to indicate things that were fixed.

If a commit has been made that changed a line of code that previously had a comment associated with it, that comment will be marked as outdated. It will disappear from the Code sub-page and will appear as collapsed on the Discussion sub-page. By doing that, Beanstalk removes comments that are no longer relevant so you can focus on what’s important.

Repeat the cycle of reviewers posting issues and developers fixing those issues for as long as it’s necessary to illuminate all problems with the branch.

Approve the branch

When all the work is done, all the code is reviewed, all the bugs fixed and all the tests completed, the branch can be marked as approved. It makes sense to keep the review open during the testing phase on staging to make sure that all fixes for issues that were discovered during testing are reviewed as well.

To approve a branch, an assignee must click the big green Approve button at the bottom of the Code Review page. By default, only one assignee is required to approve the review in order for it to be considered approved, even if multiple persons were assigned to it. You can change that behavior in Code Review Settings to require every assignee to cast their approval before a review can be closed.

A Reviewer can choose to automatically merge the branch after it has been reviewed by using a checkbox above the Approve button. If the checkbox is already checked then the person who requested the code review has requested this branch to be merged after it’s approved.

There are two things you should note about closing reviews. It’s not possible to approve a branch that has open issues or has merge conflicts with the base branch. A closed code review will be automatically re-opened if someone makes a new commit to that branch. This option can be disabled in Code Review Settings.

Ship it!

Approve it, merge it, deploy it then delete it

After the branch has been approved and merged, it can be deployed to production. It makes sense to delete branches that were merged and deployed after a few days to keep your Branches page clean and tidy. This completes the cycle necessary to develop high quality code and ship the end product to your users.

Oops! Implement a quick hot fix without creating a branch

In an ideal world all changes would have to be implemented in stand-alone feature branches. In the real world, however, that’s not always practical. Sometimes after the branch has been approved, merged and deployed to production a few issues are discovered and hot fixes have to be implemented right away. Sometimes making a commit directly to the stable branch is the most practical choice in such situations to save time and deliver a fix as quickly as possible. Should you not review code in such cases? Of course you should! You can review those stand-alone commits separately by using the Approve button on the Changeset page. The commit author can use comments on the page to request a review, then the reviewer can use in-line comments to create a discussion or use the big green Approve button to approve the commit.

Keeping track of code reviews

Team members should be on a constant lookout for code reviews that were assigned to them and reviews for branches for which they’re responsible. On the other hand, team leads and managers should be able to track reviews to see when certain features can be shipped to customers and if there are any problems with code quality. Beanstalk provides a great way for everyone to track what they want.

On the Dashboard there’s a sidebar widget that shows a number of open code reviews per repository that are assigned to you. This is useful to team members that are conducting code reviews since they will have reviews assigned to them.

The Branches page inside of a repository shows the review status for each branch. It makes it easy to see if a specific feature or bug fix is ready to be released or if it’s even in review yet. This is useful to developers and team leads who want to know if a certain branch is ready to be merged.

The Reviews page inside of a repository provides a variety of filters that allow you to find code reviews that are approved or pending, assigned to certain team members or unassigned, that were requested by you or some other team members, and more. Custom filters can be saved as bookmarks for quick access later.

Team leads, for example, might create a bookmark to keep track of all unassigned reviews or reviews that are assigned to the Interns team to keep an eye on their progress. Beanstalk provides a few default bookmarks to quickly find reviews that are assigned to you, were requested by you, or simply all reviews.

To keep an eye on the reviews that have been in progress for too long, Beanstalk has a Stale Reviews section on Reviews page. A review is considered stale if it wasn’t approved one month after it was requested.

Conclusion

Reviewing each other’s code is a necessary step in any modern development team’s workflow. Just like a book author wouldn’t publish a book without an editor’s review, a developer should never release work without having it reviewed first.

Code Review with Beanstalk is built to make this process seamless and encouraging. Your team has a common goal, get your work out to your users on time and bug-free. Having a solid code review process is the most important step to get that accomplished.

We hope this guide helps you build a code review process with your team. Let me know if you found it helpful or if you want to share a story of how you integrated code reviews into your team’s workflow.