tags:

views:

262

answers:

4

How much time do you spend doing peer code reviews?

Do you review everything?

I'm trying to push for more code reviews where I work at the moment and am trying to get a feel for what happens in the wider world.

+4  A: 

My take is that you should review at a high level early in development, focusing on design and architecture, and review at a lower level late in development, focusing on efficiencies and technique.

See also

When is the most effective time to do code reviews?

Code review process?

Ed Guiness
+1  A: 

While it would be nice to code review everything, it's difficult to get folks to put in the proper time (especially those new to formal code reviews). Start with informal reviews (over the shoulder reviews, reviewing checkins, peer coding some, ...) Then add more formal reviews on important blocks of code.

kenny
+1  A: 

At my previous place, we had a chunk of time devoted to code reviewing all new code. It was a formal affair and we typically had a few reviewers look at each piece. We followed some metric - something like 250 lines reviewed per hour and broke things up into chunks that took two hours or so. It could be a long, drawn out affair for certain pieces of code. I had one application that had around 18K LOC and so you can imagine the hours devoted to looking that over.

In retrospect, I'm not convinced that the weeks of review were always time well spent. At the time, we didn't do unit tests and we spent a LOT of time fiddling around with UML diagrams in Rational Rose. I'm not convinced that doing that was worthwhile either.

At my shop, we have a less formal code review process, but I do a LOT of testing in-house and probably have 3x the lines of test code that I do production code. That is the piece of the equation, at least for me, that seems to be the best bang for the buck. Having a bunch of tests verify that things work as advertised is essential, I think. But, it is a philosophy that some folks don't buy into.

We're in the code that each other writes pretty regularly but we don't have formal code reviews. I think it is a cultural thing and I honestly think the code we write at my shop is better cleaner and more correct code than the stuff that we wrote at my previous place, despite the lack of a formal review process. It might be the caliber of the developer we have here. We just do things differently here and it seems to work well.

With that said, however, if you are going to do code reviews. Budget time for them. Don't make them an afterthought or try to cram them into some schedule. Devs might do a pretty shallow look at the code if they are pressed for time and overloaded trying to get "real work" done. When we didn't budget well for it, we ended up getting little utility out of them.

Just my thoughts.

itsmatt
+1  A: 

1.) Code reviews have to be planned and accomodated in the schedule.

2.) Code Overview/Pre-view is what we do. i.e. author of the code first explains the functionality of the code he wants to be reviewed. Algorithm level. This gives a good idea and background for the reviewer and he gets various perspectives towards the code during the actual code review viz: functionality perspective, optimization(execution time/memory) perspective, portability, readability

3.) At one meeting only 2 hours of code should be reviewed. We found this by trial and error that 2 hr is kind of limiting case(not to less, not to much) keeping the quality of reviews/review comments

4.) Send the code to the reviewers 2-3 days in advance, based on the complexity/LOC of the code, after the pre-view has happened.

5.)Reviewers review it offline and come to the meeting with their comments.

6.)In meeting author listens to each review comment, accepts/rejects it with the associated reason

This process has served me ok till now.

-AD

goldenmean