Most of the advice seems to amount to watering down one's criticism. If we do that, won't the result be accepting less-good code into our projects? Is that the outcome we want?
It is often hard to tell the difference between great code and bad code. I've written code that was fast and very flexible, at the expense of being complex - the flexibility turned out not used at all (after 8 years in production I'm confident it never will be), so on hindsight this wasn't great code - but if the flexibility envisioned had been used the complexity would be required and the solution would be great. I've seen code that cleverly handled multi-threaded issues such that can't be understood or modified by anyone without breaking - but it great code because it abstracts the threads so nobody else has to understand them.
Second, when you first start on a project you don't understand it all. We want to grow your understanding and bring you in as trusted contributor. Setting the right tone in those first days makes all the difference in the question of will you contribute more code or not.
Last, most open source projects are essentially unmaintained. Almost any contribution is better than that, even if the code is bad, if it works at all...
At my previous company, the lead dev did this (the complex code part with the lack of usage too), unfortunately his view was that it might be used sometime.
I think, after 10 years, we were handling about 100 events per second peak load and what I pointed this out he countered with: What if we had to handle 100000 or 1 Million?
I gave up after that.
For context we were a B2B company with a relatively long sales cycle, so ramp ups were generally very strongly correlated with on-boarding of new customers and we normally had notice of what was coming. In other words, the naive solution would've worked fine ( the 100 events (not web) were across ~ 5 servers (servers did other stuff too)) and we would've been able to scale up while we did improvements to the naive solution.
Such as:
> Be kind. Don't be snarky. Comments should get more thoughtful and substantive, not less, as a topic gets more divisive.
or
> Please don't post shallow dismissals, especially of other people's work. A good critical comment teaches us something.
and
> Please respond to the strongest plausible interpretation of what someone says, not a weaker one that's easier to criticize. Assume good faith.
Being kind doesn't mean being a push over. Rather, it makes it easier to achieve the goals you want.
None of these suggestions advise you to accept less-good code. Rather, it's all about making it easier to get better code.
I could have also just said "Can't you see that you are obviously wrong?" and left it at that. Would that have been better?
Rephrasing criticism is a pretty common technique not exclusive to code reviews and imho helps avoid a defensive stance which makes it easier to be productive.
Proper review really must have "X because Y" form with perhaps exception for trivial syntax fixes. Like form 5.
Part two also forces additional burden of proof on the author. Generally for performance changes documentation requirement is the only thing. Preferably runtime numbers or profiles rather than abstract notions. We've seen people going for algorithmic gold plating where it actually list performance to constant factors.
Doing 6 in excess is viewed as patronizing. There is no good way around it.
And finally, presuming you know the teammates making the review as bland as possible without a bit of color can be the worst thing as reviews will be felt as a chore. They are one still but humor can help. Some people like their reviews to be ultimately super boring instead, and you have to be careful.
I've personally never seen anything as bad as the examples given (I've read stuff from Linus that are there, but seems like he's trying to change the atmosphere there now), but if the communication is that prickly, I can't imagine it is going to receive a ton of outside help in the form of pull requests (that could be intentional though).
I may be wrong but I read the article as a senior reviewing a junior's code, in which case the big win is not just getting better code in now, but getting a better developer. Starting the conversation instead of jumping all over them is important.
Even when reviewing a peer, while more terse we tend to phrase things in questions, "Shouldn't this be in a lib instead?" Frequently it's oversight on their part, but sometimes they have their reasons and I learn something in return.
(or possibly to avoid being sheldon in the big bang theory)