Code Review Culture
Disclaimer: if you are reading this article only because it is part of a nice blog, stop reading. If you are reading this article only because you want to know why there is such a thing called code review or what the benefits of it are, stop reading.
Now you can proceed. Happy reading and do not forget to share your ideas in the comments below!
Culture is, in the words of E.B. Tylor, “that complex whole which includes knowledge, belief, art, morals, law, custom and any other capabilities and habits acquired by man as a member of society.” This has a lot to do with code review. Knowledge and experience which is reflected in the code under review and in the comments attached to it during the review ceremony. Belief – there are subjective topics addressed, such as method names, use of synonymy etc. Of course, most of the other elements of the above enumeration are and can be mapped, like law could be the formalized coding and design guidelines, especially those which cannot be ensured by static code analysis.
Looking at the activity of code review from a very high-level perspective, we can put it into two categories: formal and informal. To put it in other words: the Good (informal, people-oriented), the Bad (still informal, but tool-driven) and the Ugly (formal).
- The Ugly: even if formal code reviews are necessary from time to time in order to comply with different governmental regulations or different enforced standards, it should happen as rarely as possible. Having a huge overhead and lacking most of the original intention of code reviews, it should not be the default policy. It would oppose to getting fast, iterative feedback on the incremental, small code blocks which are added to the code base.
- The Bad: informal code reviews are way better as they are lightweight, easy to instantiate, even ad-hoc. However, if it is tool-driven, i.e. the tool, whichever it might be, is the one shaping the process of review, it can become clumpy. Communication will be slow, mostly in form of not very well curated comments. Pre-commit review becomes impossible. Eventually, reviews get left behind, contributing to a not-so-nice chubby technical debt to be.
- The Good: the cream of code review would be informal, with short feedback loops, where communication takes place through the most efficient channels with instant code review follow-up. No code review should be left pending for anything longer than a coffee break, as it would prohibit sharing the code, a.k.a. pre-commit code review. Of course, there is no such thing as perfect. However, we should strive to get as close as possible.
Some techniques that could help us get close(r) would be the following: pair programming or over-the-shoulder reviews, and using some repos for some level of asynchrony, such as pastebins can also help. Noting down ideas, especially conclusions, observing patterns and incorporating generalized forms of them in the team culture (or even a coding guideline wiki) helps prevent the same mistakes/unwanted code. There are lots of these polishes which can help. Share yours in the comments below!
If there is only one thing you want to remember about good code review, it should be that code review is as good as how deep it made it into the team culture.
There is also a pre-commit code review technique which can help reduce the number of issues found during team code review. Let’s call it auto code review or self-code review. It’s like rubber duck coding. Before sharing the code with others (before push, commit etc. depending the tools you use), take a look at the diffs and review it just like it would have been written by some of your peers. Reading it outside of the IDE or taking a coffee break before doing so could help. It is scientifically proven that muttering also helps a lot!
What about you? How is code review performed on the project you work on? What is your personal preference?
Tags: Tech Thoughts