[Development] Code Review of Build File Changes

[Volker Hilsheimer]

> On 21 Jul 2022, at 00:30, Thiago Macieira <thiago.macieira at intel.com> wrote:
> 
> 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.
> 
> 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.
> 
> 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.
> 
> I also recommend you see how documentation-only changes are reviewed, and 
> ditto for unit-test-only changes.


It would perhaps be interesting to know how you measure the “likelihood for a change to be discussed". Is it the average number of comments given to a change? Or the number of patch sets it takes before a change gets approved and merged?

These would be observable, quantifiable measurements. And if one of the reason for those measurements is “there are very few people who can review build system changes”, then that’s a qualitative statement, getting to which seems to be why Mattie is asking for people to sign up.

Although if that would be the reason, I’d rather expect reviews to take longer, rather than the discussion to be less. But given that the main contributors to the build system share an office location, perhaps we learn that e.g. pair programming and face to face discussions reduce the amount of time it takes to get changes through review.


Volker