Saturday, October 25, 2008

Not Exactly Correct, More Its Opposite

One of the problems with code reviews is that the recipient of the review can get a little, uh, defensive. Assuming that the review isn't combative and makes valid points, why would this be? After all, a bit of constructive criticism should always be welcome.

While this is more or less true, consider that day to day life of a developer in consultant software projects consists of talking to various groups for clarifications and any other information required, writing code, and finally, being on the receiving end of something a bit like this:



First, the project manager lets you know that your estimate of two months was wrong, because you've only got six weeks. Then your own code or someone else's tells you over and over again that you're wrong (unit tests, compilers, etc). Next the code goes to the testers, people hired full time to explain in detail just how wrong you are. After this, it's off to the client project team for another round of testing. Finally, it goes live, the end users get their hands on the software and you can be damn sure they'll tell you that you're wrong. Guess which frustrated question you hear from end users?
  1. "Who designed this crap?"
  2. "Who signed off on this crap?"
  3. "Who wrote this crap?"
Hint: most end users don't know that there's a design phase or sign off.

Yes, this is an exaggeration, but all I'm saying is that maybe developers are not the prima donnas
they seem to be when they overreact to code reviews. Maybe it's just that adding yet another group of people to tell them they're wrong can be a bit annoying the first few times.

1 comment:

jml said...

Yeah, good point.

There are two ways we try to mitigate this on Launchpad:

1. The reviewer must always be grateful for the code they are reviewing.

2. Before we start work on a new branch, we have a one-on-one "pre-implementation call", where we thrash out how the work should be done. Generally, the call takes place between the developer and the person who will be the reviewer.

This means that the reviewer is way less likely to find *big* things to critique. If they do, then they'll at least have a good understanding of why you made the decisions you did.

Note: I did a straw poll of Launchpad developers at the recent Epic in London. "Pre-implementation" tend to be done mid-implementation.