On Friday, 20 April 2012 at 20:55:58 UTC, H. S. Teoh wrote:
The problem is, code review is a full-time job if it is to be
done
properly. Often it just deteriorates into a side-job: senior
coder A
reviews junior coder B's code, but coder A has a pending
deadline that
week and has lots of complex code issues to tackle, so there's
no time
to read through everything that coder B wrote. So just glance
over it
briefly, if there are no blatant problems, approve it.
Actually, a good code review should last about an hour, no more.
Studies show that in average, the great majority of problems are
seen in this hour, which is the point of diminishing return.
Moreover, it's not useful to have the code reviewed by more than
one person. What that means is, the change reviewed cannot be
more than 300-400 lines long, else, the change is probably too
big anyway.
But what's important is, the reviewer must be up to the task.
It gets worse if senior coder A himself doesn't have a good
grasp of
good coding practices, but got to where he is because he's just
very
good at last-minute patchwork hacks that makes everything
"work".
Seniority gives him code review rights, but he's really not
qualified
for the job.
If the Senior programmer doesn't have good coding practices, then
the entire exercise is counter productive. So basically, code
riew must be done by trusted persons. That means about one out of
five developers in a team.
Problem is, good coders are hard to come by. It's even harder to
distinguish between coders who succeed because they practise
sound coding principles, and coders who "succeed" because
they're good at hacking things at the last minute to make them
work.
T
Yeah, in the end, it's a question of trust.
