Click here to monitor SSC
SQLServerCentral is supported by Redgate
 
Log in  ::  Register  ::  Not logged in
 
 
 


Code Reviews Must Be Brutally Honest


Code Reviews Must Be Brutally Honest

Author
Message
hakim.ali
hakim.ali
Say Hey Kid
Say Hey Kid (694 reputation)Say Hey Kid (694 reputation)Say Hey Kid (694 reputation)Say Hey Kid (694 reputation)Say Hey Kid (694 reputation)Say Hey Kid (694 reputation)Say Hey Kid (694 reputation)Say Hey Kid (694 reputation)

Group: General Forum Members
Points: 694 Visits: 1031
Comments posted to this topic are about the item Code Reviews Must Be Brutally Honest

Hakim Ali
www.sqlzen.com
mtucker-732014
mtucker-732014
SSC-Enthusiastic
SSC-Enthusiastic (101 reputation)SSC-Enthusiastic (101 reputation)SSC-Enthusiastic (101 reputation)SSC-Enthusiastic (101 reputation)SSC-Enthusiastic (101 reputation)SSC-Enthusiastic (101 reputation)SSC-Enthusiastic (101 reputation)SSC-Enthusiastic (101 reputation)

Group: General Forum Members
Points: 101 Visits: 369
No need for brutality - you can be honest without being brutal
Core6430
Core6430
Valued Member
Valued Member (51 reputation)Valued Member (51 reputation)Valued Member (51 reputation)Valued Member (51 reputation)Valued Member (51 reputation)Valued Member (51 reputation)Valued Member (51 reputation)Valued Member (51 reputation)

Group: General Forum Members
Points: 51 Visits: 207
mtucker-732014 (8/6/2012)
No need for brutality - you can be honest without being brutal


I 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.
hakim.ali
hakim.ali
Say Hey Kid
Say Hey Kid (694 reputation)Say Hey Kid (694 reputation)Say Hey Kid (694 reputation)Say Hey Kid (694 reputation)Say Hey Kid (694 reputation)Say Hey Kid (694 reputation)Say Hey Kid (694 reputation)Say Hey Kid (694 reputation)

Group: General Forum Members
Points: 694 Visits: 1031
Core6430 (8/6/2012)
mtucker-732014 (8/6/2012)
No need for brutality - you can be honest without being brutal


I 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
Sergio Pines
Sergio Pines
Forum Newbie
Forum Newbie (7 reputation)Forum Newbie (7 reputation)Forum Newbie (7 reputation)Forum Newbie (7 reputation)Forum Newbie (7 reputation)Forum Newbie (7 reputation)Forum Newbie (7 reputation)Forum Newbie (7 reputation)

Group: General Forum Members
Points: 7 Visits: 50
Personally, I find two things show professional dedication: experience/knowledge and the ability to laugh at oneself; in the majority of cases the latter carries the most weight for me. I say this not to underscore once again what everyone here is saying, that the reviewee should act professionally and quit childish "it's my toy" reactions, but rather to point out that the reviewer should, too. Although in the majority of cases, these reactions come up in reviewees, unfortunately I have seen them crop up in reviewers as well - who insist on questioning issues that nobody cares the least about (because they are simply irrelevant); who seem to get a power-rush from being in a position to show off how far and how deep their Vision cuts (although it may appear pathetic to the rest). Curiously, when you poke at them there, their reactions are exactly the same as those of the childish reviewee: they throw a tantrum or blacklist you. Hence, I'd say that "the attitude" holds at both ends of the reviewer-reviewee relation. It means being as brutal with oneself as one is to others, in the same measure as one is able to fondly laugh about oneself as about others.
adlan
adlan
SSC Rookie
SSC Rookie (29 reputation)SSC Rookie (29 reputation)SSC Rookie (29 reputation)SSC Rookie (29 reputation)SSC Rookie (29 reputation)SSC Rookie (29 reputation)SSC Rookie (29 reputation)SSC Rookie (29 reputation)

Group: General Forum Members
Points: 29 Visits: 96
I think 'frank and fearless' is likely to be way more constructive than 'brutal'. If you have a group of people in a room, all pointing out shortcomings in someones code it is pretty inevitable that they are going to get defensive, especially if they haven't had a chance to think about the criticism ahead of time. Code reviews are probably going to be more productive if you comment code ahead of time and give the person a chance to think about it before discussing it with them one on one. You can always bring in others if you can't agree. I suppose the format needs to be informed by the personalities of those involved.
Robert.Sterbal
Robert.Sterbal
SSC Veteran
SSC Veteran (233 reputation)SSC Veteran (233 reputation)SSC Veteran (233 reputation)SSC Veteran (233 reputation)SSC Veteran (233 reputation)SSC Veteran (233 reputation)SSC Veteran (233 reputation)SSC Veteran (233 reputation)

Group: General Forum Members
Points: 233 Visits: 2000
How big is the organization you work for? This might not be an appropriate process for everyone involved. Do you have a manager that you both report to?
hakim.ali
hakim.ali
Say Hey Kid
Say Hey Kid (694 reputation)Say Hey Kid (694 reputation)Say Hey Kid (694 reputation)Say Hey Kid (694 reputation)Say Hey Kid (694 reputation)Say Hey Kid (694 reputation)Say Hey Kid (694 reputation)Say Hey Kid (694 reputation)

Group: General Forum Members
Points: 694 Visits: 1031
Sergio Pines (8/6/2012)
Personally, I find two things show professional dedication: experience/knowledge and the ability to laugh at oneself;

...

Hence, I'd say that "the attitude" holds at both ends of the reviewer-reviewee relation. It means being as brutal with oneself as one is to others, in the same measure as one is able to fondly laugh about oneself as about others.


Absolutely agree, well said.

Hakim Ali
www.sqlzen.com
hakim.ali
hakim.ali
Say Hey Kid
Say Hey Kid (694 reputation)Say Hey Kid (694 reputation)Say Hey Kid (694 reputation)Say Hey Kid (694 reputation)Say Hey Kid (694 reputation)Say Hey Kid (694 reputation)Say Hey Kid (694 reputation)Say Hey Kid (694 reputation)

Group: General Forum Members
Points: 694 Visits: 1031
adlan (8/6/2012)
I think 'frank and fearless' is likely to be way more constructive than 'brutal'. If you have a group of people in a room, all pointing out shortcomings in someones code it is pretty inevitable that they are going to get defensive, especially if they haven't had a chance to think about the criticism ahead of time. Code reviews are probably going to be more productive if you comment code ahead of time and give the person a chance to think about it before discussing it with them one on one. You can always bring in others if you can't agree. I suppose the format needs to be informed by the personalities of those involved.


Frank and fearless, or brutally honest, I think we are using different words to say the same thing. The point I am trying to make is that the criticism should be directed at the code, not the person. We are trying to improve the product/service, not prove anybody wrong.

Hakim Ali
www.sqlzen.com
hakim.ali
hakim.ali
Say Hey Kid
Say Hey Kid (694 reputation)Say Hey Kid (694 reputation)Say Hey Kid (694 reputation)Say Hey Kid (694 reputation)Say Hey Kid (694 reputation)Say Hey Kid (694 reputation)Say Hey Kid (694 reputation)Say Hey Kid (694 reputation)

Group: General Forum Members
Points: 694 Visits: 1031
Robert.Sterbal (8/6/2012)
How big is the organization you work for? This might not be an appropriate process for everyone involved. Do you have a manager that you both report to?


About 150 people at the local office. Yes, we do have a very competent manager.

Hakim Ali
www.sqlzen.com
Go


Permissions

You can't post new topics.
You can't post topic replies.
You can't post new polls.
You can't post replies to polls.
You can't edit your own topics.
You can't delete your own topics.
You can't edit other topics.
You can't delete other topics.
You can't edit your own posts.
You can't edit other posts.
You can't delete your own posts.
You can't delete other posts.
You can't post events.
You can't edit your own events.
You can't edit other events.
You can't delete your own events.
You can't delete other events.
You can't send private messages.
You can't send emails.
You can read topics.
You can't vote in polls.
You can't upload attachments.
You can download attachments.
You can't post HTML code.
You can't edit HTML code.
You can't post IFCode.
You can't post JavaScript.
You can post emoticons.
You can't post or upload images.

Select a forum

































































































































































SQLServerCentral


Search