devbeatitude.com
16 Oct 2018

Code Reviews Make for Social Programming

A long time ago on a project long dead, I setup a code review system. At that time I was naive: the code review started after the code deployed to the development server. If it seemed to work, the project manager was happy, and no one really cared about the code reviews. So I switched our project from Subversion to Git, and setup a git-flow model, where code lands in feature branches and must be reviewed before it gets deployed anywhere.1

As I am not a project manager or a software development manager or a scrum master, I have limited influence over how my programmers actually work, but I do exercise despotic control over the code itself. This one change (enforcing code reviews) is the best thing I ever did in terms of code quality.

First, it lets everyone learn from each other. I learned about Java 8 streams and lambdas from reviewing other people’s code. Some of the other guys have learned from my comments.

Second, sometimes we catch real issues before the code lands. At least twice in the last two weeks, code reviews revealed a completely wrong path taken by the programmer, or a complete misunderstanding of the requirements. Now, it would be really nice if we caught these issues even earlier, like before the programmer invests days going down the wrong path. Still, better catch it in the code review, than after customer deployment.

Obviously the reviews are only as good as the reviewers. On another project in my company, “code reviews” consist of reviewers checking out the feature branch, running it locally, and seeing if it works… so they are just duplicating the work of the system tester. They are not reviewing for correctness, coding standards, security, efficiency, architectural adherence, etc.

Other times we review according to our history with the programmer. I trust some of my programmers more than others. This week, my most senior programmer sent me a short code review. I reviewed it very quickly and accepted it. During the build some unit tests failed! The test failures were from really sloppy code this programmer ordinarily would never have turned in, and that I would never have accepted, if it came from a more junior guy that I didn’t trust so much.

Good judgment in code reviews is a precious commodity. A couple of my programmers are very good and thorough code reviewers. If I had another couple reviewers like that, our product would be much better.

Footnotes:

Tags: lessons-learned
Other posts
Creative Commons License
devbeatitude.com by David Miller is licensed under a Creative Commons Attribution-ShareAlike 3.0 Unported License .