The big problem with code review is they are hard to do right and easy to do wrong. I can look at code and see that you have the brace in the right place and the correct number of spaces. I can see if you misspelled a variable (I'm personally a bad speller, but I know a lot of people who can so I'm going to claim this skill for discussion even though I personally lack it - I guess this is a "royal I") There are a number of similar things that should be delegated to automated tools run before you even start the review.
However to read the code and figure out if it works right? that is hard. Logic errors - rarely found. Code that works but will be hard to update to the next requirement - good luck seeing that.
I think thinking of code reviews along a scale might be beneficial here, if the two ends of that scale are "code review" and "feature review". A new hire, intern or any competent developer can look at some code, irrespective of context, and conduct a decent "code review", spotting problems in the code and aesthetic issues like you mentioned (spacing, misspelling etc).
A long time hire with experience on the code base would conduct more of a "feature review". Does the code address the problem or feature correctly? Is the code adhering to feature requirements? These are things that are answered towards the "feature review" of this code reviewing scale.
The thing is that code reviews are defined as a single action that encompasses that entire scale, making it so that regardless of experience or knowledge of the codebase, reviews by any developer is given the 'ok' once all issues have been resolved.
And this entire thing only applies to code reviews within organizations. Outside of those its even more of a wild west.
> A long time hire with experience on the code base would conduct more of a "feature review". Does the code address the problem or feature correctly? Is the code adhering to feature requirements? These are things that are answered towards the "feature review" of this code reviewing scale.
It is too easy even when you have been there for years to not do that. Feature reviews are hard and boring. You can look at code, but to understand it is hard and takes time. Much easier to just glance at it, hit approve and move one. I catch myself doing that all the time.
3 comments
[ 4.7 ms ] story [ 18.2 ms ] threadHowever to read the code and figure out if it works right? that is hard. Logic errors - rarely found. Code that works but will be hard to update to the next requirement - good luck seeing that.
A long time hire with experience on the code base would conduct more of a "feature review". Does the code address the problem or feature correctly? Is the code adhering to feature requirements? These are things that are answered towards the "feature review" of this code reviewing scale.
The thing is that code reviews are defined as a single action that encompasses that entire scale, making it so that regardless of experience or knowledge of the codebase, reviews by any developer is given the 'ok' once all issues have been resolved.
And this entire thing only applies to code reviews within organizations. Outside of those its even more of a wild west.
It is too easy even when you have been there for years to not do that. Feature reviews are hard and boring. You can look at code, but to understand it is hard and takes time. Much easier to just glance at it, hit approve and move one. I catch myself doing that all the time.