3 ms·

I'm not sure if this is related but maybe egoless programming is what I am thinking about here... I felt uneasy reading the OP. I follow much of the advice myse

I'm not sure if this is related but maybe egoless programming is what I am thinking about here... I felt uneasy reading the OP. I follow much of the advice myself, and I can see how it is necessary to be diplomatic when there are tensions in the team. However, I much prefer it when people give me their raw and direct thoughts on the code. "indentation wrong", "really hard to follow this code", "why does this class exist?"; now we're two developers looking at code, and we both want it to be as neat as possible. If I can tell that the reviewer goes out of their way to be diplomatic (e.g. by following the OP or by randomly praising code that is not broken), they're obviously worried about hurting me; and suddenly we are not managing my code, but me! Why? Am I on the brink of being fired? Did I ever overreact? Am I too attached to my source code? It also becomes impossible to tell how strongly the other side actually feels about their suggestions when everything is posed as a "what do you think" question. It could be a cultural thing too? I'm German and American politeness often makes me paranoid because every flowery statement could be criticism in disguise.

mpweiher 9y ago
Very much this. > American politeness What's interesting is that Americans don't notice/understand that their expectations are not universal in the least bit, but very, very culturally specific, not shared by most of the rest of the world and, quite frankly, rather broken.
burntsushi 9y ago
Thanks for at least providing examples. From my perspective, you didn't really understand the OP. The point isn't to use "flowery" language. The point is to use more constructive feedback to avoid wasting valuable time and resources. > It could be a cultural thing too? I'm German No. I work with people all over the globe in open source. In my experience, humans everywhere appreciate helpful honesty.
jdmichal 9y ago
I think that all your example comments are entirely appropriate. And, in the case of, "Why does this class exist?", I think it's extremely important that such questions are asked. In my opinion, knowledge transfer is one of the primary functions of code review in the first place. And that question is as much about knowledge transfer as it is about the code. Maybe there's a consideration that led you to create that class, and this is a chance for that other person to learn it. And maybe you will learn something back about another potential solution. I think the real negative that needs to be avoided is using code reviews to attempt to force people to do things exactly the way you would do them. I think many of the articles points kind of go towards this, but indirectly. Diversity of thought and approach is a good thing, and learning to live code that's not done in the fashion that I think is ideal was a lesson that took quite a while for me. EDIT: And, to be clear, "Why does this class exist?" is a potential lead-in to such a problematic viewpoint. The problem happens when, after explanation of the class' existence, the response turns into, "You should remove this class." That's where it transitions away from knowledge transfer and into forceful monoculture of approach.

mpweiher 9y ago

Very much this. > American politeness What's interesting is that Americans don't notice/understand that their expectations are not universal in the least bit, but very, very culturally specific, not shared by most of the rest of the world and, quite frankly, rather broken.

burntsushi 9y ago

Thanks for at least providing examples. From my perspective, you didn't really understand the OP. The point isn't to use "flowery" language. The point is to use more constructive feedback to avoid wasting valuable time and resources. > It could be a cultural thing too? I'm German No. I work with people all over the globe in open source. In my experience, humans everywhere appreciate helpful honesty.

jdmichal 9y ago

I think that all your example comments are entirely appropriate. And, in the case of, "Why does this class exist?", I think it's extremely important that such questions are asked. In my opinion, knowledge transfer is one of the primary functions of code review in the first place. And that question is as much about knowledge transfer as it is about the code. Maybe there's a consideration that led you to create that class, and this is a chance for that other person to learn it. And maybe you will learn something back about another potential solution. I think the real negative that needs to be avoided is using code reviews to attempt to force people to do things exactly the way you would do them. I think many of the articles points kind of go towards this, but indirectly. Diversity of thought and approach is a good thing, and learning to live code that's not done in the fashion that I think is ideal was a lesson that took quite a while for me. EDIT: And, to be clear, "Why does this class exist?" is a potential lead-in to such a problematic viewpoint. The problem happens when, after explanation of the class' existence, the response turns into, "You should remove this class." That's where it transitions away from knowledge transfer and into forceful monoculture of approach.