[Development] Code Review of Build File Changes

[Edward Welbourne]

On Wednesday, 20 July 2022 11:29:57 PDT Mattie Nejati wrote:
>> I’m Mattie, a Ph.D. student at the University of Waterloo and I’ve
>> been studying the code review process of build files in Qt. For
>> example, I’ve found that changes to build files are 2 to 4 times less
>> likely to be discussed during code review than changes to source code
>> or test files.

Thiago Macieira (21 July 2022 00:30) replied
> I dispute the hypothesis. You haven't proven it, and you're now
> proceeding to find out why something is before ascertaining that it is
> so.

You also need to control for complexity of changes.  I will hazard a
guess that a significant subset of changes to build files are simple
changes such as adding a new source file to a list of source files; a
second significant subset will be adding a new sub-directory to some
project, with a fairly straightforward build config that's a copy of
some other sub-directory with trivial details changed.  It is no
surprise that these get no comment in review.

So you need to distinguish trivial changes to build configuration (to be
compared with trivial changes to source code, of which there are also
several categories) from Real Work on the build system, which defines
new CMake functions or reworks existing CMake functions to fix bugs or
extend their capabilities.  These last are then comparable to the Real
Work changes to source code (that, if they touch the build system at
all, typically do so in trivial ways).

In short, when comparing how likely different categories of change are
to be discussed in review, the first question to ask is how complex the
change is; and you'll need to control for that when comparing changes in
different categories.  In particular, a commit that mixes changes of
different categories may be complex in one but trivial in the others.

> There are two people who can review build system changes. You should
> find other code areas that only have one or two people who can ever
> review them (or, worse, zero) and there's a similar symptom there.

and you'll likely find those two people do most of the non-trivial
change to the build system, and review each other's work, at which point
Volker's observation that they share an office may be relevant.  In
contrast, the two people who do most (and review each other's)
non-trivial changes to date-time code sit on opposite sides of the
Atlantic, so you'll see in review discussions the build team would have
had across their desk.

	Eddy.