[Chandler-dev] Reviewing code you don't know

[Alec Flett]

Heikki Toivonen wrote:
> To go beyond that, you'll probably need to talk to the person whose code
> you are reviewing. This does take time, which you may not always have.
>   
I'd like to expand on this further, and bring back my metaphor of 
driving. You can drive at 100mph and crash into things now and then, or 
drive at 50mph and not hit anything. If you hit something hard enough at 
100mph, you're not going anywhere for a long time.

When you're driving 100,000 miles it absolutely makes sense to drive 
100mph for much of that just because you've got a lot of ground to 
cover. But when you get off at a rest stop, you slow down to refuel, get 
your bearings and make sure you're headed in the right direction. That 
means slowing down and being careful. You don't want to spend 24 hours 
at a rest stop because you hit some guy in the parking lot... :)

Real, honest to goodness code reviews absolutely slow down the volume of 
code and checkins that happen, but dramatically reduce regressions and 
this is vital just before a release.

Reviews also have the added bonus of spreading knowledge around. I can't 
claim to be an expert on the repository, but listening to Andi explain 
his rationale for some of the collection/set changes he's made lately 
has really improved my knowledge of notifications, sets, and some of the 
lower level repository stuff.

Alec

> There is added benefit to the original author in explaining the code; it
> is not unusual for them to spot an error when explaining the code to
> others. The reviewer should get a reasonable level of understanding of
> the code immediately around the changes to be able to review those lines.
>
>   
> ------------------------------------------------------------------------
>
> _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
>
> Open Source Applications Foundation "chandler-dev" mailing list
> http://lists.osafoundation.org/mailman/listinfo/chandler-dev
>   

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.osafoundation.org/pipermail/chandler-dev/attachments/20060504/1aa8ecd5/attachment.htm