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.

Code Review Culture

Courtesy of Google Images. Too many results to find the original creator of the Uncle Sam painting from which this image was derived.

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).

  1. 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.
  2. 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.
  3. 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.

Code Review Quote

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!

Rubber Duck Coding

What about you? How is code review performed on the project you work on? What is your personal preference?

Tags:

One comment

Great, concise article! My take-away here confirms the view that I’ve had the last few years: Code review tools are far from mature!

+1 for auto-reviewing. I have made it a habit to review my code in git gui during staging (I usually find things like “Oh, that’s still there? Better remove it.” and “Oops, spelling error.”).

Also +1 for “pre-commit” reviews. Fixing issues after the fact has a much higher threshold.

Another important thing: Automate as much as possible to let human reviewing focus on design rather than format. For example, do automatic linting and static analysis (and the likes) as part of the code review. Also use automatic code formatting.

My wish for a review tool that would motivate tool-driven reviews:

1) It should integrate well with the work flow (GitHub, GitLab, Gerrit and Critic come close, Collaborator and Review Board – not so much).
2) It should integrate well with other tools (build & test, issue tracker, integrate/deploy, …).
3) It should make life easier for both the developer and the reviewer (keeping track of what has been reviewed and addressed etc), rather than being a clumsy speed-bump in the process.
4) In the case of Git (my DVCS of choice), the review tool should support common Git concepts such as history rewrite, and reviewing the commit history (multiple commits per review, commit comments), etc.

Leave a Reply

Your email address will not be published. Required fields are marked *