LWN.net Logo

Humphrey: On Code Review

Humphrey: On Code Review

Posted Jan 21, 2013 16:56 UTC (Mon) by NAR (subscriber, #1313)
Parent article: Humphrey: On Code Review

I remember that about a decade ago code review here meant that people printed copious amount of code to dead tree, disappeared into a meeting room for hours, then emerged in a foul mood (the result of bickering about the necessary number of spaces) - and then nothing happened with the results, because the code review was scheduled only after delivery, due to everyone being busy with last minute fixes. All this made the "code review" expression dreaded.

In recent projects we've simply setup the infrastructure to send an e-mail about each commit, so the interested parties can review it (hopefully in time). It is not mandatory, but I think a very useful tool not only to spot errors, but to keep up to date with remote parts of the codebase.


(Log in to post comments)

Humphrey: On Code Review

Posted Jan 21, 2013 17:07 UTC (Mon) by marduk (subscriber, #3831) [Link]

I too remember a similar process (nearly two decades past) whereby all code was printed and had to be reviewed by a senior-level person. If the submitter were also senior level (or happened to be particularly friendly with the reviewer), then she probably didn't even look at the code and quickly signed it off. If the submitter were at a lower level (or an advesary of the reviewer), the code might be meticulously picked apart. There were no safegaurds to ensure that the reviewer was actually fit to review the code in the first place other than she had been with the company long enough to get "senior" status.

Humphrey: On Code Review

Posted Jan 21, 2013 23:53 UTC (Mon) by marcH (subscriber, #57642) [Link]

Enforcing any process or tool is pointless as long as the majority fails to understand the value of it. A process should only (try to) organize things people genuinely want to put some effort in.

Finding time for code reviews is hard enough when people agree they are useful; no surprise they're not done properly when people miss the point.

If you work in a company that does not even understand the value of code reviews then you should probably try to find a better job where quality, teamwork and personal development actually matter. Run even faster if a rigid process forces code reviews to happen anyway.

Humphrey: On Code Review

Posted Jan 22, 2013 2:02 UTC (Tue) by rgmoore (✭ supporter ✭, #75) [Link]

Enforcing any process or tool is pointless as long as the majority fails to understand the value of it. A process should only (try to) organize things people genuinely want to put some effort in.

I'm not sure if I completely agree. Sometimes people have to experience something to appreciate the benefits, so enforcing it as a matter of policy has to come before general acceptance. That is admittedly a high risk approach. It will only work if the people implementing the new policy understand the benefits and do a good job of enforcing it in a way that will eventually make the benefits clear to everyone else. Simply trying to enforce an idea, even a good idea, without a clear understanding of the way it's supposed to work is asking for failure.

Copyright © 2013, Eklektix, Inc.
Comments and public postings are copyrighted by their creators.
Linux is a registered trademark of Linus Torvalds