Code review is just the last line of defense to make sure people don't merge things that will create problems for everyone else.
Good engineers figure out the important stuff before anyone writes a line of code. The APIs, the architecture, high-level component names. It's all been talked over. When a good engineer reviews another good engineer's pull request, they're just checking that what is delivered is an implementation of what is expected.
Code review is a road block for people who don't do the above. It causes them to bump into someone who knows what's up.
Everything else that gets argued about: local variable names, formatting, equivalent ways to express the same thing. It's not important. I don't know if it's actually confusion about what matters, or if it's a desire to make people jump through hoops, or maybe boredom?
I mostly agree with you, but often, especially with greenfield projects and features, the implementation details are not clear until someone spends the time to dig into the codebase, and deliver a proof-of-concept. This can then drive the design discussion to get everyone on the same page. And even then, the design might need to be changed if it happens that some things weren't taken into consideration, or new specs are communicated and priorities change.
It's also worth not spending too much time on creating the perfect design upfront, since it might change during development. The whole waterfall vs. agile situation. An initial proposal document helps with avoiding lengthy discussions and friction during code reviews, but it also takes a lot of effort to produce, and even then, reviews can be difficult for many reasons.
> I don't know if it's actually confusion about what matters, or if it's a desire to make people jump through hoops, or maybe boredom?
My theory is that sometimes it's a need to say _something_ to prove that you've read the code. Also, egos are often involved, and some reviewers have the need to demonstrate their superior intellect/knowledge/whatever by insisting on minutia. Software engineers are often proud and take code reviews as a chance to flaunt their intellect, while at the same time being defensive of their opinions, and argumentative to the point of arrogance. Few senior engineers are humble, and consider code reviews as a collaborative effort to deliver the best possible solution.
Good engineers figure out the important stuff before anyone writes a line of code. The APIs, the architecture, high-level component names. It's all been talked over. When a good engineer reviews another good engineer's pull request, they're just checking that what is delivered is an implementation of what is expected.
Code review is a road block for people who don't do the above. It causes them to bump into someone who knows what's up.
Everything else that gets argued about: local variable names, formatting, equivalent ways to express the same thing. It's not important. I don't know if it's actually confusion about what matters, or if it's a desire to make people jump through hoops, or maybe boredom?