It depends on the scope What are you reviewing? You can't possibly check all changes for bugs, or that they meet the Acceptance Criteria. That's the developer's and QA's job to do.
Key takeaway: You have to speak with your team and agree what the scope of your code reviews should be.
For me, I limit myself to reviewing code conventions:
- Formatting. In some projects we use pre-commit hooks to deal with formatting,. In others we don't, and I've seen some horrible stuff. For me that's grounds to put the PR in the penalty box, because when the next developer does something in the same area and they format the code, they will prevent the previous developer's commit message being immediately accessible.
- Weird / inefficient expressions. For example, someone doing a bunch of loops and if-statements when the same can be done with Java 8 streams.
- Common pitfalls for your language. For example in Java I always flag
str.equals("constant") and ask them to change it to
"constant".equals(str). The OWASP list of common vulnerabilities is an excellent source for this, and they have sections for all major languages.
- Documentation. Flag any public interfaces that aren't documented, or if the documentation does not agree with the code.
- Naming conventions. We have them, and we have them for a reason, and PRs is the place to make sure they're followed.
- Library abuse. If I'm familiar with a library, internal or external, I will make sure it's used properly. This is the closest I come to your review style, and not everyone can do this on every PR.
- Magic Constants Applies to all languages but I've seen it in SCSS more than others. For example, setting the element colour to a raw hex value and not using a variable.
- Language keys Same as magic constants above. My current project is multi-lingual and sometimes we forget ourselves and hardcode UI labels when we should be fetching them via language keys. Pretty common issue, easy to spot.
- Others. I'm sure there are other things that escape me now.
I should also mention, a lot of these things can be cut down by using a linter (ie. for CSS, JS) or static code analysis (ie. Sonar for Java, C#).
At one time in my current project (when we first started doing code reviews) the procedure was to download the branch and test it against the Acceptance Criteria on the ticket. That didn't last long because it created a huge backlog, so, don't make more work for yourself than you should.