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.
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.
>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.
Computer Games are art and games are one result of computer science. Can a science create an art? I guess color theory can explain why we enjoy colors. But the science came after the art.
I agree that data structures and algorithms are not art, but when combined in unique ways they create art.
I think the problem is the field is so new and these terms aren’t well defined.
Good point about also extending some grace when doing a review. In my experience insensitive and unnecessary, pedantic and overall shitty critique is a much bigger issue and I'd rather see people push back more on it actually.
> 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.
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.
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.
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.
> 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.
Real example #1:
I warn John Doe that his new endpoint will crash in a specific scenario. John Doe dismisses the warning since "it's not likely to happen in the wild". QA call it out soon after it's uploaded to our test environment. The error has a chain effect where it prevents them from testing other stuff.
Real example #2:
I warn John Doe that we just committed a flaky test to the develop branch, and I submit a PR to fix it. John Doe closes the PR since "for now we must accept tests fail in mysterious ways".
Soon after, Doe Johnson who works in another team is blocked by said test. He spends an hour or two coming up with the same solution I did.
In both cases we burnt money needlessly, because John Doe is too stubborn to accept our team's code is not perfect.
"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.
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.
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.
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.
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.
I very commonly catch inadvertent quadratic or even exponential algorithms in code reviews. The code normally will produce the right answer ... eventually.
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?
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.
> 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.