How To Make Your Team’s Code Better
Improving the overall code quality is a challenge. You work really hard at writing quality code, but bugs still slip in and code isn’t quite up to standards each and every time. It’s frustrating to you that there’s not a way to catch some or all of these issues beforehand; you’d love the input of others and the satisfaction that having your code reviewed brings. But how do you get a code review process started?
On my team at Mozilla, we review each and every patch before it’s committed to master. The process is simple: the developer commits their code to their branch on Github, issues a pull request to master, asks the person who they would like to review it, and waits for their patch to be reviewed. This system is simple but effective in ensuring that code is of high quality and reduces the number of bugs that go into master by putting another set of eyes on the code.
Here’s how you can get a code review process going for your team.
Make the process as simple as possible.
Complex processes are doomed to be discarded, not adopted. If your process is complex and difficult to implement for your developers, they’ll resist using it, and their resistance will almost surely doom it. Make the process easy to follow, and as few steps as possible.
Use Github pull requests and private repositories.
Github has the best user interface for code reviews I’ve seen. By being able to annotate individual lines and leave code comments, you have the ability to truly review code line by line. The pull request process ensures that developers can have their code easily integrated after review is complete. Private repositories are a cheap way for your company or team to have the user interface of Github without making your code public.
Spread out the review work so nobody is overwhelmed.
The hardest part of a code review process is having only one or two people that can review the code. Even if only one or two people have commit access, it’s important that most if not all of the team be able and assigned to review other developer’s code. Not only does it improve a developer’s skills to read through another developer’s code, it encourages team development by having developers work together. Plus, by integrating the whole team, no one or two people feel overwhelmed.
Have a weekly triage of requests needing review.
Review needs sometimes fall through the cracks. This has happened on our team from time to time and we have solved this by adding a weekly triage and review of pull requests during our weekly status meeting. This doesn’t need to be done more than once a week as developers are likely to request review and follow up on important bugs; it’s usually the big, difficult patches that don’t end up getting reviewed in a timely fashion. This weekly check-in will help.
Have a simple system for approved and rejected.
On our team, a pull request is either “r+” meaning “approved”, or “r-” which means “not approved.” An “r+” can come with considerations: “r+ with the following nits to be fixed before merging…” for example. An “r-” should ALWAYS explain why the item was rejected and how it can be fixed (if it can be fixed). Having any more of a complex system would frustrate developers.