Code reviews, the process of showing your hard work to another developer and having them tear it to shreds, are often an unloved part of the modern development cycle. The project manager asks why you would change the working solution, the tester refuses to test something twice, and you have to make your colleague admit on closer inspection their masterpiece has an awful double chin.
However it doesn’t have to be that way; code reviews can become part of the daily process and you and your colleagues might start looking forward to showing each other work and sharing knowledge. In my current role I get to do a lot code reviews and have picked up a few tips along the way.
Improving your code review technique
Before launching into code reviews with folks who are not used to them, start with pairing on a hard problem instead. The offer of help on a challenging piece of work tends to get a more welcoming response than a dreaded review and helps build trust between team members. With luck as time passes folks will ask for pairing and reviews throughout the development cycle rather than someone having to insist but this cultural shift takes time.
Share code reviews around the team - you can learn a lot from looking at someone else’s code and sharing knowledge and experience between everyone improves team skill. There is nothing wrong with less experienced developers reviewing more senior colleagues work - especially if they haven’t been a reviewee in a while!
Code review small pieces of code and do it often. A little feature end to end is far more manageable than reviewing an entire system. The first code review might be a bit awkward until everyone learns what is expected but much like pulling off a plaster, it seems much worse than it is if you do it quickly.
Tooling can help
Git (or another DVCS) can make reviews easier because comparing changes locally is faster. Decent tooling can also assist the reviewer, using something like GitHub’s Pull Request or a tool like Phabricator allows discussions to be annotated against lines of code. It also allows the reviewee to submit the request asynchronously and get on with other stuff while waiting for review.
When reviewing with a tool like a pull request/diff I find thinking in terms of an individual’s patch against master/trunk, combined with easy diff-ing and small atomic commits make reviewing code far easier and less onerous on the reviewer so these practises should be encouraged.
A mixture of in person reviews and web based reviews balances time taken, interruptions and code quality in the team. If doing it in person the reviewer should make notes during the review, write them up and send them afterwards. That way things don’t get forgotten and the reviewee gets a tangible result from the review rather than a list of criticism.
It should be obvious but be positive and constructive at all times - it is not an opportunity show off or shred someones confidence. Concentrate on both delivering the feature to a high standard.
Seven review topics
When doing a code review I try and evaluate the change in seven areas and give feedback on each:
- Code follows standards and conventions set out in your developer guidelines (you do have those right!?) or has only minor deviations.
- Code has clear acceptance criteria & meets them fully
- Code has good automated test coverage
- Code is presented in such a way that it was easy to review
- Any post review refactoring can be completed within 25% of the original effort.
- Code has no major security or performance issues
- Code is continuously integrated, QA’d and follows your branching/integration strategy.
So code reviews can be positive for both reviewer and reviewee if you approach them right and use them as a constructive opportunity to share experience and deliver quality. Don’t forget how nerve wracking the first one you experienced was.
Further reading: A list of things to watch out for in a code review.
