DEV Community

Discussion on: How do you code review?

Collapse
 
eljayadobe profile image
Eljay-Adobe

If I'm very familiar with the code in question, I approach the changed code in the review with a critical eye with how it changes the behavior.

If I'm not familiar with the code in question, I do what you listed: style issues, code placement, lament over functions that do way too much. (We basically don't have tests. But, yes, there should be tests. My code base has plenty of awful non-linear functions that are between 10,000 and 100,000 lines long, with cyclomatic complexity that can only be properly expressed with transcendental numbers.)

Management believes that code reviews will transmit subject matter expertise through osmosis. But in my experience, code reviews do nothing of the sort except give management a warm fuzzy that they are facilitating and encouraging knowledge transfer.

To get knowledge transfer, you either need to be in a mentor/mentee situation or pair programming. Both of which also entail continuous code reviews.

Also, if the code review is small, I can assess it well. But as it gets larger, it becomes difficult to see problems except at a gloss level. I saw one tool (from Google, iirc) size code reviews as wee (1), tiny (<5), small (<30), medium (<100), nominal (<300), large (<1000), freakin' huge or mondrian (<2000), jupiterian or jovian (<3000), month long (<4000), whopping big (<5000), mother of all code reviews (<10000), grandmother of all code reviews (<20000), category 5 code review (anything larger).

Where "gloss" are things like suggesting better named identifiers, misplaced tabs if the team convention is spaces (or vice versa), misplaced curly braces from the team convention, suggesting a breadcrumb comment for some confusing logic. Those are basically superficial things.