Somehow once you get to a certain scale it becomes its own defense. "Oh we can't police that, we're really just that big, don't you know". But the small guys better watch out, no excuses for them.
Also as an dependency canary: when your low level object tests start demanding access to databases and config files and networking, it's time for a think.
Also a passing unit test always provides up-to-date implicit documentation on how to use the tested code.
> In many/most cases, there are N approaches, and you happened to pick one.
I'm not sure that's always true. A lot of the time, while there might be, in a vacuum, many ways, in the context of a specific project (or part of), there's usually one "most right" way to do it, considering local style, idioms, norms and conventions. Admittedly, this might be more true in some languages and applications.
Sometimes there are multiple ways. Usually this means that one or more disparate legacy ways are extant, but there's a preferred way to move towards.
Having PRs storm off in "exciting" new directions, technically correct or not, is usually a bad idea once that merges and is now everyone else's problem.
If all alternatives are equally valid logically, it's likely they have practical implications that differ. Perhaps one might consider a std::set better than a std:: vector in the abstract for your task, but maybe you know something important about the expected number of items or memory access patterns. This is when you should comment if that's not obvious to someone who isn't literally just done writing the change.
> At the end of the day, the burden is on the reviewer to specify why he prefers X.
If a reviewer is going to pull me up on truly equivalent things that have no impact on correctness or maintainability, then I'm probably not going to voluntarily be a co-maintainer with them. If I've consented to be a co-maintainer, then I have either mutual respect for the others, or I'm being paid enough to make it worth it. And indeed, I have refused maintainership offers in projects with ":eyeroll: why didn't you just magically intuit X" cultures.
Argh, yes, that's one line that might make me just walk away from a PR as a new/junior/casual contributor.
You, the reviewer, are an expert in the system. Likely you are the or one of the most expert people in the entire world on this exact thing. You know X exists and why to use it. As you should, because you put it there. You also should know that people who aren't experts (like me) don't know about it, simply because they didn't use it, in this PR, when they should have. Why don't they know it? Probably because you haven't used it consistently in your own code, or it's not documented. This newbie has cobbled this PR together from what sense I can make of this project. Probably 90% is guessed from code I found in there already.
What wouldn't wind me up?
"I think a better way to do this is X. It's better because Y. Or have I missed a specific reason for X?"
Note two things: 1) explanation to a noob of reason Y, which may well be valuable, not only to the noob, but also in the record of the project in general. 2) The indication that the noob might at least have had a logical approach, and they're not an idiot, just a noob.
Afterwards, if this seems like something the noob should have known from the codebase, consider that you, the maintainer, have failed to make it clear.
Of course, if I'm also supposed to be an expert, e.g. a co-maintainer, then it's different. I should know X. Which means either it's a brain fart, or I actually do have a reason. In which case I should have commented on the code, because if another contributor can't tell the intention in the PR, then they can't tell in a year when no one can remember why it went that way.
> The traditional finance system is no more energy efficient overall.
Using more power[1] to process vastly more transactions makes it more efficient. There are over a billion credit card transactions per day as it is, and that doesn't include debit cards, wire transfers, BACS, Swift, PayPal, WU, AliPay, WeChat and all the others. And cash while we're at it, that's part of the system too.
[1]: Lets say it does, though transaction processing is only a small part of the financial industries. I don't even know how much the whole industry uses, let alone what fraction is transaction handling.