If ever there is a time to avoid bikeshedding consciously, it is during code review. Easy nits are of proportionally relatively small value. It's not misspelled comments or "not quite the variable name I would have picked" that brings down systems in general.
One thing I depend on in my reviews is that the programmer has indeed verified that it does what it says it does, preferably via some unit testing. That frees me to consider architecture things and whether there's a lurking bug without having to review whether it works. This has been particularly helpful in our Infrastructure-as-Code reviews. Many such reviews are post-facto; they've already been run, we know it brings up the right infrastructure already, and the review is to make sure it's architecturally heading in the right direction and maybe that tagging standards weren't accidentally violated and other high level, important things, not whether we like the particular name of a subsystem.
Getting this balance is probably the hardest thing about code reviews.
Also, one of the main things in code review is having extra eyes validate that nobody's putting obvious back doors in the code. If you've done that, you've brought some value even if you do nothing else. If a bug gets past the person authoring the code, it's not a huge shock that it'll get by someone who is not in that headspace, doing something else, and has a completely different context loaded. The idea that it wouldn't borders on magical thinking. But at least you don't see any "eval(base64_decode('big long string'))" today. The goal of reviewing is just to up your team's game a bit, not guarantee on their own that no bugs will make it to QA or prod or customers. They're part of the portfolio and shouldn't be treated as anything more.
One thing I depend on in my reviews is that the programmer has indeed verified that it does what it says it does, preferably via some unit testing. That frees me to consider architecture things and whether there's a lurking bug without having to review whether it works. This has been particularly helpful in our Infrastructure-as-Code reviews. Many such reviews are post-facto; they've already been run, we know it brings up the right infrastructure already, and the review is to make sure it's architecturally heading in the right direction and maybe that tagging standards weren't accidentally violated and other high level, important things, not whether we like the particular name of a subsystem.
Getting this balance is probably the hardest thing about code reviews.
Also, one of the main things in code review is having extra eyes validate that nobody's putting obvious back doors in the code. If you've done that, you've brought some value even if you do nothing else. If a bug gets past the person authoring the code, it's not a huge shock that it'll get by someone who is not in that headspace, doing something else, and has a completely different context loaded. The idea that it wouldn't borders on magical thinking. But at least you don't see any "eval(base64_decode('big long string'))" today. The goal of reviewing is just to up your team's game a bit, not guarantee on their own that no bugs will make it to QA or prod or customers. They're part of the portfolio and shouldn't be treated as anything more.