
Ask HN: What makes for effective code reviews? - kd22
How do you conduct code reviews? What process do you follow?
======
LandR
One process I've used in the past which I thought was quite succesful was:

1) Entire team must do code review. Even if it's just a member clicking
"Decline, code review done already by LandR"

2) We used 3 levels.

* Finished, Looks Good - This is good, you can check it in to our dev branch.

* Finished with comments - This is good, but I have a few comments, if you make the changes in teh comments its good to check in to our dev branch.

* Finished, Needs Work - This code is bad. There is a big issue here and you need to look at it / rewrite it. The new code will need a new code review. DO NOT check in what you have.

All check ins were gated and no check ins allowed unless everyone has either
finished the review with Finished Looks Good or finished with comments.

If there was a dispute, e.g. 4 Finished with Comments and 1 Finished Needs
Work. Then you would need to have a conversion with the Finished needs Work
person (or if they are right, ignore the results from the others).

This process kept code quite tight.

What you found was though the more junior members were almost always Finished
Looks Good,because they couldn't recognise issues. The problem was though,
afterwards when they had done their "finished looks good", they didn't look at
the code review again..

If you can get the more junior members involved in everyones code review and
get them to look at comments on >other peoples code< from the more senior
members it can be a good learning opportunity.

------
bluehatbrit
Our peer reviews are pretty standard, we have the same check list for everyone
though anyone is always welcome to suggest changes to it (which are discussed
separately to an individual peer review).

We also always try to frame feedback in regards to the code and project, and
not the author. This attempts to make things less personal and helps us focus
on the task rather than the individual completing it.

I guess the only other important thing to note is we always try to mix up
review pairs / groups. We don't have that many people so this is very easy,
but it means that we keep a nice mixed pot of ideas and thoughts rather than
ending up in silos where Bob and Alice only review each others work and no one
else gets overview of it.

Oh, we also try to make sure we pick out good things as well as changes we'd
like to see. This is just generally encouraging and stops reviews being "what
can you do better" and makes them more of a true look at the work as a whole.
"Oh I love how you abstracted this module, it's made your code here much
easier to understand" often makes people feel good about the work and gets
them excited to make the few small changes the rest of the team thinks should
happen.

