Live data from Hacker News

Code reviews aren’t just for catching bugs

blog.fullstory.com

1–10 of 150 posts

Re: Code reviews aren’t just for catching bugs

#2
I'd still rather earlier code reviews... design reviews about 1/3 of the way into writing the code. Enough time to have passed to have discovered the dragons in the whiteboard design, but not enough to have written code that could only undergo minor fixes in a code review.

I prefer the idea of a code review happening at a time that it could still steer the ship... too many code reviews catch bugs, but don't correct poor system design. That's also where the greatest benefit/education exists for the author... not minor changes, but "what about approaching it like this".

Re: Code reviews aren’t just for catching bugs

#3
post #2

I'd still rather earlier code reviews... design reviews about 1/3 of the way into writing the code. Enough time to have passed to have discovered the dragons in the whiteboard design, but not enough to have written code that could only undergo minor fixes in a code review. I prefer the idea of a code review happening at a time that it could still steer the ship... too many code reviews catch bugs, but don't correct p…

This is generally why smaller changes are better. They are more easily reviewed, and more easily "steered" as you put it by a good review.

And of course having a design doc ahead of time that the team can review and comment on for a longer, more complicated change, is a great thing to do. And compliments the subsequent code review[s].

Re: Code reviews aren’t just for catching bugs

#5
post #2

I'd still rather earlier code reviews... design reviews about 1/3 of the way into writing the code. Enough time to have passed to have discovered the dragons in the whiteboard design, but not enough to have written code that could only undergo minor fixes in a code review. I prefer the idea of a code review happening at a time that it could still steer the ship... too many code reviews catch bugs, but don't correct p…

This is the role of design documents which get heavily reviewed by the team before even the first line of code is written.

Re: Code reviews aren’t just for catching bugs

#6
post #2

I'd still rather earlier code reviews... design reviews about 1/3 of the way into writing the code. Enough time to have passed to have discovered the dragons in the whiteboard design, but not enough to have written code that could only undergo minor fixes in a code review. I prefer the idea of a code review happening at a time that it could still steer the ship... too many code reviews catch bugs, but don't correct p…

I work on a very small team (4 engineers), so we have all commits for all branches posted to our engineering chatroom.

Everyone knows what everyone else is working on (and we all work in the same room), so when someone is on a task that we know my have some snags/difficult-to-design solutions, we'll all periodically take a break and look at decisions other people are making on their particular feature, providing feedback when appropriate.

Of course, since we all work in the same room, we can also use the WTF/minute metric to see (well, hear) when one of us is deep in the weeds and could use a second pair of eyes to pull us out.

[EDIT: and I know this won't scale too far beyond a team of our size, but for us it works quite well right now]

Re: Code reviews aren’t just for catching bugs

#7
I would go further and say that you should never trust code reviews to catch bugs. They're great for all the reasons in the article and what "zhemao" said but they're horrible for catching any bugs beyond the most trivial ones. Humans simply aren't good at running code in their head like that. Code review is no substitute for good code coverage and automated testing, preferably pre-commit.

Re: Code reviews aren’t just for catching bugs

#9
post #8

Requiring code reviews before the commit is a bureaucratic waste of time and resources. If they non-mandatory and after the commit, then they can be a good idea.

wow. how so? You write perfect code and there is no need for a second pair of eyes ? This type of arrogance is always puzzling to me.

Re: Code reviews aren’t just for catching bugs

#10
post #8

Requiring code reviews before the commit is a bureaucratic waste of time and resources. If they non-mandatory and after the commit, then they can be a good idea.

After commit there is little incentive to do the review (code is already there, QA can test it, story is done etc.)
Post reply on HN