To me, the problem this incident is pointing out is the toxicity of code review.
We speak about changing the name of a local variable used four times in the whole codebase. Was this that a big deal? Merge it already and be done with it. Easy way to make people happy to contribute.
Instead of that, people had to comment that change and explain why they don't like it, leading to confrontation, frustration and the departure of a team member. This is so typical.
The worst part in code review is that people are convinced they do the right thing. How being aware of changes and trying to do some peer work could be a bad thing? Well, it's basically commenting over and over other people work, leading authors to mistrust themselves and to the feeling of unfairness. Reading other people code is cool. Commenting it... well, you should think you have a very good reason to do that.
Are you proposing that the ability to comment on proposed changes is toxic? How exactly is a group of people supposed to come to a consensus on technical matters without the ability to communicate? Are you really suggesting that certain changes proposed by certain people get special treatment, immunity from discussion, and expedited merging to mainline?
Technical excellence in such an environment is impossible. Nobody's stopping you running a project that way, but if you tried to introduce this kind of discrimination, prejudice, and, well, privileged treatment into a project to which I contribute, I'd fork, and everyone who actually wanted to write code would follow me.
Your comment calls "code review", the practice, "toxic". You go on to suggest that project maintainers "merge [the pull request] already and be done with it", suggesting that the normal review process not apply to this particular diff. You suggest that there should be a high bar for commenting on other people's code.
Your comment sounds to me like it advocates lowering the bar for merging of external patches. That's a sure way to technical inferiority.
> Your comment calls "code review", the practice, "toxic".
Oh also, I think it should be clarified.
I'm not advocating code review is a bad thing in itself and we should stop doing it. I think it could be done better, by internalizing the impact of a comment and measuring its cost before doing it.
I see doing code review as better than not doing it (because you read other people code). I see being moderate when it comes to decide if something is worth commenting as better than the usual practice of considering comments are cheap and we can just comment anything.
> Your comment calls "code review", the practice, "toxic". You go on to suggest that project maintainers "merge [the pull request] already and be done with it"
I'm with you until there :)
> suggesting that the normal review process not apply to this particular diff
If the normal review process is to feel diff should absolutely be commented, then yes, totally, applying it to such a small and casual diff makes what I feel is the ridiculousness of the process totally shine.
Note that I'm not saying the diff should not be read. I'm not saying either we should not comment other people code if we feel there is a problem with it. I'm challenging the fact that this diff should have generated discussion, and call the fact that it had a problem.
I don't think making teams more efficient lead to technical inferiority. And to me, the abuse of commenting code is all about inefficiency (regarding both productivity and sane relations between team members).
We speak about changing the name of a local variable used four times in the whole codebase. Was this that a big deal? Merge it already and be done with it. Easy way to make people happy to contribute.
Instead of that, people had to comment that change and explain why they don't like it, leading to confrontation, frustration and the departure of a team member. This is so typical.
The worst part in code review is that people are convinced they do the right thing. How being aware of changes and trying to do some peer work could be a bad thing? Well, it's basically commenting over and over other people work, leading authors to mistrust themselves and to the feeling of unfairness. Reading other people code is cool. Commenting it... well, you should think you have a very good reason to do that.