Re:Rietveld - code review Google style
by btilly on Friday February 11, 2011 @01:50PM
(#2207631)
I'm sorry, but I think that your team has this backwards.
If you wait until a branch is done, then reviewers are being asked to look at big chunks of code. That is a lot more effort, and it is harder to get reviewers to do it. And when they do do it, it is harder to get them to do a thorough job of it. Furthermore when they notice design issues, it is harder to get developers to go back and redo all of their working code to make it better.
Therefore if you want effective code review, you need to review early and review often. The smaller a change is, the easier it is to review, the easier it is to be thorough, and the easier it is to accept suggested changes.
My conclusion is therefore that choosing to do code review after the fact means that you are guaranteeing no proper code review. But when you review early - literally on every commit - it is then much easier to maintain a truly effective code review process. It initially "feels" much heavier. But after you get in the rhythm, your code will be much, much improved.
Re:Rietveld - code review Google style
by dkl on Friday February 11, 2011 @03:19PM
(#2207976)
We use gerrit at my company, and what you say is true. Developers are told to do one feature or bug fix per commit/gerrit change. It really does help with the review process and things move much faster.
If you wait until a branch is done, then reviewers are being asked to look at big chunks of code. That is a lot more effort, and it is harder to get reviewers to do it. And when they do do it, it is harder to get them to do a thorough job of it. Furthermore when they notice design issues, it is harder to get developers to go back and redo all of their working code to make it better.
Therefore if you want effective code review, you need to review early and review often. The smaller a change is, the easier it is to review, the easier it is to be thorough, and the easier it is to accept suggested changes.
My conclusion is therefore that choosing to do code review after the fact means that you are guaranteeing no proper code review. But when you review early - literally on every commit - it is then much easier to maintain a truly effective code review process. It initially "feels" much heavier. But after you get in the rhythm, your code will be much, much improved.