The review gap

Did you know...? LWN.net is a subscriber-supported publication; we rely on subscribers to keep the entire operation going. Please help out by buying a subscription and keeping LWN on the net.

The free-software community is quite good at creating code. We are not always as good at reviewing code, despite the widely held belief that all code should be reviewed before being committed. Any project that actually cares about code review has long found that actually getting that review done is a constant challenge. This is a problem that is unlikely to ever go completely away, but perhaps it is time to think a bit about how we as a community approach code review.

If a development project has any sort of outreach effort at all, it almost certainly has a page somewhere telling potential developers how to contribute to the project. The process for submitting patches will be described, the coding style rules laid down, design documents may actually exist, and so on; there is also often a list of relatively easy tasks for developers who are just getting started. More advanced projects also encourage contributions in other areas, such as artwork, bug triage, documentation, testing, or beer shipped directly to developers. But it is a rare project indeed that encourages patch review.

That is almost certainly a mistake. There is a big benefit to code review beyond addressing the shortage of review itself: there are few better ways to build an understanding of a complex body of code than reviewing changes, understanding what is being done, and asking the occasional question. Superficial reviewers might learn that few people care as much about white space as they do, but reviewers who put some genuine effort into understanding the patches they look at should gain a lot more. Reviewing code can be one of the best ways to become a capable developer for a given project.

It would, thus, behoove projects to do more to encourage review. Much of the time, efforts in that direction are of a punitive nature: developers are told to review code in order to get their own submissions reviewed. But there should be better ways. There is no replacing years of experience with a project's code, but some documentation on the things reviewers look for — the ways in which changes often go wrong — could go a long way. We often document how to write and contribute a patch, but we rarely have anything to say about how to review it. Aspiring developers, who will already be nervous about questioning code written by established figures in the community, are hard put to know how to participate in the review process without this documentation.

Code review is a tiring and often thankless job, with the result that reviewers often get irritable. Pointing out the same mistakes over and over again gets tiresome after a while; eventually some reviewer posts something intemperate and makes the entire community look uninviting. So finding ways to reduce that load would help the community as a whole. Documentation to train other reviewers on how to spot the more straightforward issues would be a good step in that direction.

Another good step, of course, is better tools to find the problems that are amenable to automatic detection. The growth in use of code-checking scripts, build-and-test systems, static analysis tools, and more has been a welcome improvement. But there should be a lot more that can be done if the right tools can be brought to bear.

The other thing that we as a community can do is to try to address the "thankless" nature of code-review work. A project's core developers know who is getting the review work done, but code review tends to go unrecognized by the wider world. Anybody seeking fame is better advised to add some shiny new feature rather than review the features written by others. Finding ways to recognize code reviewers is hard, but a project that figures out a way may be well rewarded.

One place where code review needs more recognition is in the workplace. Developers are often rewarded for landing features, but employers often see review work (which may result in making a competitor's code better) in a different light. So, while companies will often pay developers to review internal code before posting it publicly, they are less enthusiastic about paying for public code review. If a company is dependent on a free-software project and wants to support that project, its management needs to realize that this support needs to go beyond code contributions. Developers need to be encouraged to — and rewarded for — participating in the development fully and not just contributing to the review load of others.

As a whole, the development community has often treated code review as something that just happens. But review is a hard job that tends to burn out those who devote time to it. As a result, we find ourselves in a situation where the growth in contributors and patch volume is faster than the growth in the number of people who will review those patches. That can only lead to a slowdown in the development process and the merging of poorly reviewed, buggy code. We can do better that, and we need to do better if we want our community to remain strong in the long term.

