Code Review Checklist

Context
As a junior developer, who up until this point has only ever performed mediocre code reviews (I’ll admit, they’ve not been great), I figured it was time to apply a more systematic approach in hopes of providing my teammates with better feedback. So here’s the list I’ll be referring to for my current, and future, code reviews.
FYI: I work mainly on web projects.
Why Code Review?
- Give junior developers a nudge in the right direction to help them hit the ground running 🏃
- Ramp up new team members 👋
- Increase Maintainability — hammer out those issues before they materialize into technical debt 🔨
- Spread knowledge throughout the team (so you can go on vacation without having to check slack 🍹 )
- Use peer pressure to ensure higher initial code quality (see: Hawthorne Effect) 💯
- Learn more effective implementation techniques 💪
- Discover Bugs and logic errors (if you’re lucky, but this should probably be driven by testing as well) 🐞
Assumptions
- The project has a linter, which is checking for stylistic issues including, but not limited to: tabs vs. spaces, # of spaces per tab, spaces between if statements and parenthesis, the presence of blank lines, trailing spaces, misaligned indentation, etc.
- The project architecture takes into account the major security concerns.
- Off by one, terminating loops, dividing by zero, and other common errors are picked up in testing.
- We are using SASS, LESS, or something of the like to compile our CSS.
What to Check in a Code Review?
Overall
- Can I read it, and understand — more or less — what is going on?
- Does it perform its intended function? Try running it and playing around with it a bit.
- Check out the UX spec — if one exists. Does the overall structure of the code seem to align with what they want. What about future UX implementations?
General
- Is there any commented-out code which has been left behind?
- Is there any unreachable code?
- Is a new module performing too many functions? Maybe it should be split up.
- Is it over-documented? If so, this may indicate that the code is not clear enough.
- Can the framework do that? It’s better to take advantage of framework features, than to re-invent the wheel (and to shove that wheel into your project where a similar, likely more round, wheel already exists).
- Were any debug logs left in prod? Make sure they don’t get there.
- Were all the edge cases in an if-else, switch, or for-loop considered? Think 0, null, 1, etc.
Dependencies
- Was that new library necessary?
- Was that new library supposed to be global?
- Was making one class dependent upon another really necessary? Can I just inject a dependency instead?
Style
- Is the code following the overall structure and architecture of the project?
- Are camelCase, kebab-case, or snake_case used where appropriate for the project (if this is not caught by the linter)? Do class names start with upper case letters?
CSS
- Is there a class for that already — if not, if it’s used throughout the project, can I extract this class from my module and apply it else where?
- Does one of our imported libraries such as bootstrap, UIKit, etc., able to do this for us? Does our development framework give us a class, or some other method, for that?
- Are our media queries defined using defined variables for widths? Are these consistent with the rest of the project?
Variables
- Are variable names clear, meaningful, and as concise as possible?
- Are there any “magic” numbers — i.e., numbers that could be clearly named constants? 🎩
- Are there cases where enums could be used to group like values?
Reuse
- Can a certain functionality be obtained from code which already exists elsewhere in the project or imported libraries?
- Can duplicate code be wrapped into something more reusable?
Performance
- Are any observers or computed properties going to be called wayyyy too often, thereby impacting performance?
- Is it scalable? If 100,000 new users were added, is it still going to function performantly? (lol jk @ 100,000 — you’re probably not AWS, but it should be scalable within reason for the requirements of the feature and project).
- Are there any processes which may be run in parallel?
- Are there any opportunities for lazy loading?
- Would it be worth it to cache that for next time?
- Are exceptions caught and handled appropriately?
Memory, Data Flow, & Data Structures
- Are optimal data structures being introduced for the intended purpose?
- Is memory being reallocated when appropriate?
- Are connections closed correctly when a task is complete?
- Does the flow of data make sense? Parent-child module communication? Should that be in a universal store?
UX
- Were there any unreasonable UX asks that have disrupted the integrity of the project from a technical perspective? Maybe we should push back.
- Does using that feature feel weird? Maybe you should talk to your functional manager and UX about that, and try to brainstorm ways that feel less wrong. *This may be outside the scope of your CR* 😓
Translations
- Be on the lookout for missed translated strings — they’re easy to over look.
- Is the way something is formatted going to work for all languages? Left-to-right languages? What about Right-to-left (Hebrew, for example)?
- Is punctuation able to be modified for different languages? For example, comma, question-mark, exclamation point placement, if necessary, or will that be included in the translation string?
Security
- Are any authentication tokens being exposed unnecessarily?
- Are inputs being sanitized? Can the framework perform this, perhaps?
- Sensitive information is encrypted, right? RIGHT?! 😬
Testing
- Are all new modules unit tested? (yeah, once in a while you can get away without having them, but this shouldn’t be a habit).
- Do the tests clearly describe the behavior of the function/method/statements it is testing?

Finally…
If you see something that smells a little funny, ask yourself, “Would the tech lead slap me on the wrist for that?” If the answer to that question is anything but “heck no”, then you should probably call it out.
This check list could be more comprehensive, so what do you check for in your code reviews that isn’t already caught by static analyzers, or in this list? 😁