7 ms·

I agree with all of it. I find commandment 2 to be the most enlightening one. > You are not your code. Remember that the entire point of a review is to find pr

by mkl95 5y ago

I agree with all of it. I find commandment 2 to be the most enlightening one. > You are not your code. Remember that the entire point of a review is to find problems, and problems will be found. Don’t take it personally when one is uncovered. In my career, I have had to deal with other senior developers who would throw tantrums whenever I pointed out something problematic about their code. Over the years, there's something all those people seem to have in common - they are stuck in an endless loop of making mistakes and refusing to learn from them.

hutzlibu 5y ago
"Over the years, there's something all those people seem to have in common - they are stuck in an endless loop of making mistakes and refusing to learn from them." I feel this sadly describes many partnerships, as well as great parts of society and humanity as its whole. But I am optimistic, that this can change, without a big catastrophic event needed for people to wake up.
lkschubert8 5y ago
I really don't understand the attitude of getting frustrated when people correct mistakes in your code review. I would so much rather have a coworker catch my bug in a cr than have it make it to prod. One is mildly embarrassing (if you tie your ego to your code) the other has an actual impact.
- epolanski 5y ago
  The overwhelming majority of comments I read, and I wish that was true just for my company, it applies to majority of OSS as well rarely if ever catch a bug. I see most of the discussion involve preferences, practices and names, rarely api and architecture, even less bugs.
  - ScottBurson 5y ago
    It's very hard to catch bugs in a code review. I manage it occasionally, but it's much more common that I'll be working on something else that touches on some piece of code that already passed review, and then I'll notice a bug, because I'm already thinking about how that module should work -- I have already loaded the relevant context into my head. Loading that context just to do a code review takes too long.
    - not2b 5y ago
      I very commonly catch inadvertent quadratic or even exponential algorithms in code reviews. The code normally will produce the right answer ... eventually.
- tonyedgecombe 5y ago
  Human nature. Some people will use code review as a stick to beat you with or an opportunity to show how clever they are.
  - jseban 5y ago
    Yep, and this is unfortunately the rule more than the exception, and somehow part of STEM culture
- jseban 5y ago
  It all depends on the delivery. Criticising someone's work is sensitive and you need to consider this. My number one problem is always that not enough positive feedback is given, you work really hard and make a great solution to a really important problem, give it to the team, and all you get back is someone aggressively and stubbornly picking on it for minor details. It's just a really negative attitude, you go to see the Sistine Chapel with someone, and the only thing they have to say about it is that there was a crack in the ceiling, and just repeat that the crack should be fixed. Would that not affect your experience?
leetrout 5y ago
I LOVE this frame of mind and I love this statement but it is usually only used in the negative alignment of expectations: don't take critique personally. No disagreement. But let's get pedantic: The umbrella "you are not your code" would dismiss improvement as well. And praise. Ok, so I am not my code. But if I don't learn from my mistakes my code will not improve. I... I... I... Anyway, I much prefer to reframe it this way: Programming is a very intimate experience. The same as creating art. It is the manifestation of your thoughts and opinions, small and large, put out in to the world. So sure, don't be offended by critique. But also remember to extend some grace when doing a review.
- majormajor 5y ago
  > The umbrella "you are not your code" would dismiss improvement as well. And praise. I don't see why you think this is the same thing. Improvement in someone's coding skill is good from a business/colleague/project perspective. And from a "practice paid off" perspective. And positive feedback is always good to give, you certainly shouldn't only give negative feedback. But you shouldn't get too attached to your code in either case. Today's good code is likely to be tomorrow's bad legacy code anyway. It shouldn't be an "intimate experience," programming for a company is an exercise in utility.
  - jseban 5y ago
    I agree. People are not robots, this is just silly. You'd never get a good effective team by taking this extreme stance that nobody ever has any feelings about their work.
- dgfitz 5y ago
  > Programming is a very intimate experience. >The same as creating art. It is the manifestation of your thoughts and opinions, small and large, put out in to the world. I respectfully disagree. This thought process is why other people are left holding the bag at 11pm on a Friday fixing the 'art' of someone else. Computer Science is a derivative of mathematics. Math is not art. Math IS beautiful, explicitly because it is NOT ambiguous or subject to interpretation. Math is right or wrong. It works or it is flawed. Programming isn't art. edit: forgot a >
westoncb 5y ago
I think I'd have nodded and read on a couple years ago when I'd only had thoughtful/well-informed/diversely-experienced developers review my code. I've learned since that it's also possible to have your code reviewed by someone without particularly deep understanding (nor awareness of what they lack), who would insist that responding to a critique of (what they think is) a problem is essentially being argumentative because they are certain they've already got the 'correct' answers (in a similar way that a first year philosophy student might think they've already got the answers). > ... they are stuck in an endless loop of making mistakes and refusing to learn from them I am probably overfitting to my own recent experience, but to me, while this could be a legitimate problem with the reviewee, sounds equally plausibly like a red flag on the part of the reviewer: someone who has settled into a set of "correct" answers and now sees other people not adopting their personal outlook on code as a failure to learn. It is not a simple matter to (definitively, non-subjectively) find a 'problem' once the criteria become anything more nebulous than "does it produce the correct result". There's an analogous situation I've noticed in more casual conversations: someone is describing a problem they have or a situation they're in, and the person they're speaking to keeps smugly offering "solutions" that only sound good because they haven't listened closely to the other person, took a superficial glance and assumed the issue was some common one and so offered a facile/common solution—and then don't understand why the other person isn't appreciative.
- snapcore 5y ago
  Yeah, the person who thinks a mistake is "you aren't writing the code my way" in itself is an ego trap. A lot of programmers get trapped in their viewpoint and think everyone should write code like them because it would make more sense to them if everyone did, when it isn't objectively better.
  - sethammons 5y ago
    "Given the choice between my opinion and yours, I'll take mine. Got any data?" -former boss
    - skj 5y ago
ourmandave 5y ago
In my career, I have had to deal with other senior developers who would throw tantrums whenever I pointed out something problematic about their code. Obviously how and when you point it out matters. Like in the middle of a user demo. "Why does it let me enter Feb 30th?" "Oh, Dave wrote the validation on that. What was your thought process on that one Dave?"
- kwertyoowiyop 5y ago
  “Making sure our QA Department is worth their budget, Steve.”
- whatshisface 5y ago
  "Future-proofing for the introduction of leap months."
  - tonyedgecombe 5y ago
    https://www.timeanddate.com/date/february-30.html https://www.timeanddate.com/date/february-30.html
- epolanski 5y ago
  Why's there so many devs at a user demo?
- Jtsummers 5y ago
  Another way to say this, which should be common sense, is don't call people out in public unless the intent is to embarrass them. Which is, of course, an asshole thing to do. If you spot an issue, you raise it in the appropriate forum. This could be a one-on-one, feedback through whatever review system you use, or feedback in a review meeting if that's used. But calling it out in public, especially in front of a manager (more than one level above) or a customer is a great way to demonstrate that you are, well, an asshole.
jseban 5y ago
I agree in a way, but also disagree. Programming is probably the only job where you'd expect someone to be completely emotionally detached from your work. Everyone has feelings of pride and accomplishment for work that they have made, and you always need to consider this when you give criticism. It makes a world of difference if you review in a friendly and constructive way, or in a mean and aggressive way. There's no way people can just "not have any feelings" about work that they have put a lot of effort into.

by mkl95 5y ago

hutzlibu 5y ago

"Over the years, there's something all those people seem to have in common - they are stuck in an endless loop of making mistakes and refusing to learn from them." I feel this sadly describes many partnerships, as well as great parts of society and humanity as its whole. But I am optimistic, that this can change, without a big catastrophic event needed for people to wake up.

lkschubert8 5y ago

I really don't understand the attitude of getting frustrated when people correct mistakes in your code review. I would so much rather have a coworker catch my bug in a cr than have it make it to prod. One is mildly embarrassing (if you tie your ego to your code) the other has an actual impact.

epolanski 5y ago
The overwhelming majority of comments I read, and I wish that was true just for my company, it applies to majority of OSS as well rarely if ever catch a bug. I see most of the discussion involve preferences, practices and names, rarely api and architecture, even less bugs.
- ScottBurson 5y ago
  It's very hard to catch bugs in a code review. I manage it occasionally, but it's much more common that I'll be working on something else that touches on some piece of code that already passed review, and then I'll notice a bug, because I'm already thinking about how that module should work -- I have already loaded the relevant context into my head. Loading that context just to do a code review takes too long.
  - not2b 5y ago
    I very commonly catch inadvertent quadratic or even exponential algorithms in code reviews. The code normally will produce the right answer ... eventually.
tonyedgecombe 5y ago
Human nature. Some people will use code review as a stick to beat you with or an opportunity to show how clever they are.
- jseban 5y ago
  Yep, and this is unfortunately the rule more than the exception, and somehow part of STEM culture
jseban 5y ago
It all depends on the delivery. Criticising someone's work is sensitive and you need to consider this. My number one problem is always that not enough positive feedback is given, you work really hard and make a great solution to a really important problem, give it to the team, and all you get back is someone aggressively and stubbornly picking on it for minor details. It's just a really negative attitude, you go to see the Sistine Chapel with someone, and the only thing they have to say about it is that there was a crack in the ceiling, and just repeat that the crack should be fixed. Would that not affect your experience?

leetrout 5y ago

I LOVE this frame of mind and I love this statement but it is usually only used in the negative alignment of expectations: don't take critique personally. No disagreement. But let's get pedantic: The umbrella "you are not your code" would dismiss improvement as well. And praise. Ok, so I am not my code. But if I don't learn from my mistakes my code will not improve. I... I... I... Anyway, I much prefer to reframe it this way: Programming is a very intimate experience. The same as creating art. It is the manifestation of your thoughts and opinions, small and large, put out in to the world. So sure, don't be offended by critique. But also remember to extend some grace when doing a review.

majormajor 5y ago
> The umbrella "you are not your code" would dismiss improvement as well. And praise. I don't see why you think this is the same thing. Improvement in someone's coding skill is good from a business/colleague/project perspective. And from a "practice paid off" perspective. And positive feedback is always good to give, you certainly shouldn't only give negative feedback. But you shouldn't get too attached to your code in either case. Today's good code is likely to be tomorrow's bad legacy code anyway. It shouldn't be an "intimate experience," programming for a company is an exercise in utility.
- jseban 5y ago
  I agree. People are not robots, this is just silly. You'd never get a good effective team by taking this extreme stance that nobody ever has any feelings about their work.
dgfitz 5y ago
> Programming is a very intimate experience. >The same as creating art. It is the manifestation of your thoughts and opinions, small and large, put out in to the world. I respectfully disagree. This thought process is why other people are left holding the bag at 11pm on a Friday fixing the 'art' of someone else. Computer Science is a derivative of mathematics. Math is not art. Math IS beautiful, explicitly because it is NOT ambiguous or subject to interpretation. Math is right or wrong. It works or it is flawed. Programming isn't art. edit: forgot a >

westoncb 5y ago

I think I'd have nodded and read on a couple years ago when I'd only had thoughtful/well-informed/diversely-experienced developers review my code. I've learned since that it's also possible to have your code reviewed by someone without particularly deep understanding (nor awareness of what they lack), who would insist that responding to a critique of (what they think is) a problem is essentially being argumentative because they are certain they've already got the 'correct' answers (in a similar way that a first year philosophy student might think they've already got the answers). > ... they are stuck in an endless loop of making mistakes and refusing to learn from them I am probably overfitting to my own recent experience, but to me, while this could be a legitimate problem with the reviewee, sounds equally plausibly like a red flag on the part of the reviewer: someone who has settled into a set of "correct" answers and now sees other people not adopting their personal outlook on code as a failure to learn. It is not a simple matter to (definitively, non-subjectively) find a 'problem' once the criteria become anything more nebulous than "does it produce the correct result". There's an analogous situation I've noticed in more casual conversations: someone is describing a problem they have or a situation they're in, and the person they're speaking to keeps smugly offering "solutions" that only sound good because they haven't listened closely to the other person, took a superficial glance and assumed the issue was some common one and so offered a facile/common solution—and then don't understand why the other person isn't appreciative.

snapcore 5y ago
Yeah, the person who thinks a mistake is "you aren't writing the code my way" in itself is an ego trap. A lot of programmers get trapped in their viewpoint and think everyone should write code like them because it would make more sense to them if everyone did, when it isn't objectively better.
- sethammons 5y ago
  "Given the choice between my opinion and yours, I'll take mine. Got any data?" -former boss
  - skj 5y ago

ourmandave 5y ago

In my career, I have had to deal with other senior developers who would throw tantrums whenever I pointed out something problematic about their code. Obviously how and when you point it out matters. Like in the middle of a user demo. "Why does it let me enter Feb 30th?" "Oh, Dave wrote the validation on that. What was your thought process on that one Dave?"

kwertyoowiyop 5y ago
“Making sure our QA Department is worth their budget, Steve.”
whatshisface 5y ago
"Future-proofing for the introduction of leap months."
- tonyedgecombe 5y ago
  https://www.timeanddate.com/date/february-30.html https://www.timeanddate.com/date/february-30.html
epolanski 5y ago
Why's there so many devs at a user demo?
Jtsummers 5y ago
Another way to say this, which should be common sense, is don't call people out in public unless the intent is to embarrass them. Which is, of course, an asshole thing to do. If you spot an issue, you raise it in the appropriate forum. This could be a one-on-one, feedback through whatever review system you use, or feedback in a review meeting if that's used. But calling it out in public, especially in front of a manager (more than one level above) or a customer is a great way to demonstrate that you are, well, an asshole.

jseban 5y ago

I agree in a way, but also disagree. Programming is probably the only job where you'd expect someone to be completely emotionally detached from your work. Everyone has feelings of pride and accomplishment for work that they have made, and you always need to consider this when you give criticism. It makes a world of difference if you review in a friendly and constructive way, or in a mean and aggressive way. There's no way people can just "not have any feelings" about work that they have put a lot of effort into.