3 ms·

The most toxic behavior is being overly protective or offended about your code. This goes both ways: Both as the review-seeker (that opened a PR) and the review

The most toxic behavior is being overly protective or offended about your code. This goes both ways: Both as the review-seeker (that opened a PR) and the reviewer (That most of the time got his original code modified by the PR). As a general rule I think there is too much emotion in what should be 100% technically driven. And again, this goes both ways. Being judgemental in a review (like on some of the link's slides) or being offended by a technical comment on the other side. Too much sugar coating is not helping anyone

trevyn 9y ago
>I think there is too much emotion in what should be 100% technically driven. Assuming that the prospective solutions offered by each side both fulfill the basic technical requirements, the choice of two different engineering approaches would seem to be a 100% human decision about tradeoffs, no?
thedz 9y ago
I don't expect a team that is fearful of code reviews to be the best work they can do. IMO this isn't about offense, it's about removing barriers and wariness around code reviews. (But as you side, that goes both ways, to both the reviewers and reviewed)
mannykannot 9y ago
The flip side of an author who is overly attached to his code is the reviewer who is overly attached to an excessively pedantic and dogmatic view of how things should be done. As you say, it goes both ways: the former should acknowledge valid technical criticism of his work, and the reviewer should refrain from using general arguments that are not relevant for the specific case.
eldavido 9y ago
"100% technically driven". That's a silly statement. You aren't dealing with objective facts here, it's opinions. Tastes. There are always egos, personality, and differing points of view. I think a better view is art/music. When you go to art school, you learn how to take criticism well. You learn to make it about the work, and view it as a chance to learn, and get better. I try to nurture this attitude for reviews at my company.
- xfer 9y ago
  Opinions, tastes are essentially bike-shedding in programming and reviewers shouldn't be asking it to be fixed, unless there is a rulebook or automated tool for the bike-shedding. Otherwise programming decisions should always be fact/experiment driven not opinions.
kelnos 9y ago
> The most toxic behavior is being overly protective or offended about your code. I disagree with "most", but this is absolutely often an issue. Often people take criticism of their code as criticism of themselves and their abilities, which is always counterproductive. > As a general rule I think there is too much emotion in what should be 100% technically driven. In my experience, things that are 100% technically driven are vanishingly rare. Most decisions come down to balancing trade offs and opinions, after examining the actual facts and evidence at hand. Regardless, people have emotions and don't always act fully rationally, despite concerted efforts to do so. That's just a fact of human nature, and we'd do well to design our professional processes with that in mind.
- thedz 9y ago
  > I disagree with "most", but this is absolutely often an issue. Often people take criticism of their code as criticism of themselves and their abilities, which is always counterproductive. I think it's easy to fall into "this comment is a value judgement on my skill/person, and not my code" if code review feedback is targeted at the person and not the code. Hence the discouraged "why did you do this" form vs the more helpful "this might be better because" format.

trevyn 9y ago

>I think there is too much emotion in what should be 100% technically driven. Assuming that the prospective solutions offered by each side both fulfill the basic technical requirements, the choice of two different engineering approaches would seem to be a 100% human decision about tradeoffs, no?

thedz 9y ago

I don't expect a team that is fearful of code reviews to be the best work they can do. IMO this isn't about offense, it's about removing barriers and wariness around code reviews. (But as you side, that goes both ways, to both the reviewers and reviewed)

mannykannot 9y ago

The flip side of an author who is overly attached to his code is the reviewer who is overly attached to an excessively pedantic and dogmatic view of how things should be done. As you say, it goes both ways: the former should acknowledge valid technical criticism of his work, and the reviewer should refrain from using general arguments that are not relevant for the specific case.

eldavido 9y ago

"100% technically driven". That's a silly statement. You aren't dealing with objective facts here, it's opinions. Tastes. There are always egos, personality, and differing points of view. I think a better view is art/music. When you go to art school, you learn how to take criticism well. You learn to make it about the work, and view it as a chance to learn, and get better. I try to nurture this attitude for reviews at my company.

xfer 9y ago
Opinions, tastes are essentially bike-shedding in programming and reviewers shouldn't be asking it to be fixed, unless there is a rulebook or automated tool for the bike-shedding. Otherwise programming decisions should always be fact/experiment driven not opinions.

kelnos 9y ago

> The most toxic behavior is being overly protective or offended about your code. I disagree with "most", but this is absolutely often an issue. Often people take criticism of their code as criticism of themselves and their abilities, which is always counterproductive. > As a general rule I think there is too much emotion in what should be 100% technically driven. In my experience, things that are 100% technically driven are vanishingly rare. Most decisions come down to balancing trade offs and opinions, after examining the actual facts and evidence at hand. Regardless, people have emotions and don't always act fully rationally, despite concerted efforts to do so. That's just a fact of human nature, and we'd do well to design our professional processes with that in mind.

thedz 9y ago
> I disagree with "most", but this is absolutely often an issue. Often people take criticism of their code as criticism of themselves and their abilities, which is always counterproductive. I think it's easy to fall into "this comment is a value judgement on my skill/person, and not my code" if code review feedback is targeted at the person and not the code. Hence the discouraged "why did you do this" form vs the more helpful "this might be better because" format.