I like reviewing code. I often learn new tips or tricks whenever I review. It doesn't matter if it expert or novice code, I learn from each.
Keeping track of everything can be difficult when doing reviews, so here is my personal list of things I check for.
- Spelling: Spelling things correctly helps others search the code more effectively when debugging, refactoring, or writing enhancements.
- Transposed text: If it is a content update, I double check the source material.
- Links: Any relative links? Absolute links? SSL? The correct answer depends on the project but I always click through to check that they work.
- Overly complex code: I do not like code golf. Writing overly elegant code just makes it more brittle and harder to support. If I can’t understand it at 3am during an incident, I want it refactored or documented thoroughly.
- Does it meet code standards? I like code standards. If everyone knows what the requirements are, the more likely code will be accepted. Also known as, "Argue once over tabs versus spaces, decide as a team, and WRITE IT DOWN."
- Comments: Format and length depends on the team (see code standards). Some teams prefer commit messages or READMEs over comments in code. This step could be renamed, "Is it complex or messy but documented?"
- Security: Is this resource only for admins or editors? How is that checked? Are inputs sanitized?
- Run the linter: Sometime people forget.
- Run the tests: Yes, even if there is automated testing.
- Run the code: Did the change work?
- Check existing behavior: Does this change affect something else unrelated? I usually check only two or three key paths.
If everything looks good, I approve the pull request.
Reviewing code shouldn't be a chore. It is a great learning opportunity for everyone.