Core6430 (8/6/2012)
mtucker-732014 (8/6/2012)
No need for brutality - you can be honest without being brutalI agree. Honesty is good. The article makes it sound like code reviews are supposed to be a blood bath. I'm hoping that what the author meant was that honest, helpful advice was given, but the reviewee got defensive over good criticism. Not that a bunch of his peers were put in a room to shred the poor guy from one end to the other disguised as "we're helping".
There's no reason for the person giving criticism to get "sullen" if criticism is not taken. That implies ego's ARE involved. Don't expect the person with the spotlight on them to react passively when confronted by 5 other people who are using it as a way to boost their ego's. Especially a group setting. That is a sure way to set someone up for defensive behavior.
Thanks for replying. So many thoughts here:
I agree too, honesty is not just good, it is essential. Brutal honesty is not the same as brutalizing somebody, and it is not a blood bath either. What it simply is, is that if the reviewer thinks that there is a way to make improvements in the code, they should not hold back from dispensing that thought for no reason other than to be polite to the reviewee.
And yes, there is no reason for anybody in the review session to get sullen. Nor should the reviewee be passive in the face of constructive criticism: if they feel their code is right, by all means defend it. But defending something you believe in, and being defensive just to not be wrong, are two different things.
And yes on the ego boost too: just like it should not apply to the reviewee, it should not apply to the reviewer either. At the end of the day, it is about the code, not about you or me. That is why it must be honest.
Hakim Ali
www.sqlzen.com