Saturday, April 25, 2009

SmartBear: A better way to do code review ...

At OpenView Venture Partners we encourage portfolio companies to do code reviews - online, not face to face. Contrary to most agile practices where face to face communication is preferable, there are some distinct advantages to online review - no posturing, no haranguing, complete documentation of the problem, leisurely review and followup.

SmartBear Code Reviewer 5.0 has some nice features and we have some reference sites for OpenView companies.

An interesting discussion on whether code review should be done before or after committing to the build can be found on the SmartBear blog.

11 comments:

csterwa said...

Would you do this even in a situation where the teams were doing pair programming? If so, what would you get extra from this? Is there a person that you put in charge of code review outside of the company or team?

gsporar said...

@csterwa: Pair programming can be used in place of online code review, but there are some pitfalls to watch for. When two people are working that closely together its easy for them both to be so close to the code that neither of them can provide the "fresh perspective" that someone else could. Letting others outside the pair review the code will also contribute to collective ownership of the code. An approach used by some teams as a middle ground: use pairs to write/review the implementation and then have the rest of the team peer review the unit and integration tests.

blogger said...

@csterwa : pair programming is not a silver bullet ! doing pair programming for all tasks is just wastefulness ! Also some teams (distributed) can't do pair programming. I totally agree with Jeff about the advantages of online code review.

csterwa said...

Thank you for the pair programming information, gsporar. I am more interested in whether Jeff and the group think the remote code review is better than pair programming.

We do "promiscuous pair programming" at SolutionsIQ and I coach this with our clients who adopt Scrum and XP technical practices with our help. I am just wondering about the potential of remote code review and practices around it.

csterwa said...

Good day Blogger,

Obviously I have touched a nerve with you. I am not saying that pair programming is a silver bullet nor do I profess that working on "every task" is always the best way to do things. In fact, I don't.

What I was asking is what are the comparisons between pair programming and remote code review and how would remote code review work. Mostly in this situation where you have, potentially, and outside venture group doing the code review on a company's code. This is not a pair programming versus code review discussion. Just want to hear out the merits of other options to pairing.

Pair programming is quite difficult and may be the wrong approach for some teams to take on. With many teams I work with it is an amazingly helpful tool in not just the coding but also in getting better synergy around design and moving knowledge around the team. There are definitely other ways to do this and I have helped teams adopt other ways in the past, as well, when it made sense to.

I do, and would not, disagree with Jeff at all on his points, I am asking for some more information and comparison data points so that I can decide how to use the information. Thanks.

Jeff Sutherland said...

Well I think everyone agrees that no pair programming and no code reviews leads to more bugs. So everyone would agree that one or the other is a good thing to lower defects and improve the code base. Even Google mandates code reviews (even though people often don't do them). We encourage companies that we invest in to do pair programming. For those who don't we say you need to be doing code reviews because see the consequences in the Board meetings when quality is not good. The interesting question is whether pair programming but automated code online code reviews makes things better. We need someone to try pair programming with and without online code reviews and see what happens.

csterwa said...

I see some potential research in the making. Thank you for the response, Jeff.

Catherine said...

Dr. Laurie Williams has published results of experiments with distributed pair programming. This can be found here, including lessons learned:

http://collaboration.csc.ncsu.edu/laurie/Papers/XPAUDistributedP.pdf

davfive said...

We have found a very solid, free alternative at http://www.review-board.org/. It was originally developed by Christian Hammond and David Trowbridge at vmware.

We have been using it for about 6 months and are getting good results from it.

Joe Little said...

It is a Lean principle to "go to the Gemba" (where the truth can be found). So, actually looking at the code while doing a code review makes complete sense to me.

What makes less sense to me, is why do this all through technology only and "mandate" no face-to-face communication. (I am assuming a case no where FTF is do-able.)

We are people, we are animals, and FTF has a different meaning than remote. Yes, agreed, some of that "meaning" can be "bad" (eg, posturing) but that's an impediment that needs to be addressed. It seems to me that making the impediment visible (via FTF) is better than hiding the impediment (via technology).

Now, I have not tried or seen this new approach, so it is more than likely I mis-understand something.

Jason Cohen said...

It's hard to argue that:

1. Meetings take much more time.
2. Some communication/learning can take place only in meetings.
3. Some meetings get off track and waste time.

Consider this: You don't have enough time to do a meeting-review of 100% of the code check-ins. But about half our customers (I'm the founder of Smart Bear) mandate that all code is reviewed before check-in to version control, proving that you do have time to review 100% of your code online.

So a compromise is in order:Start reviews online, because often that's all that's necessary.

However, never feel constrained to keep the review online if you see that face-to-face meeting, arguing, or mentoring would be valuable.

Tools like ours should be used to save time, but at the moment the tool doesn't fit, by all means put it down!