Code Review Checklist:
- Logging
- Monitoring
- Scale
- Traceability
- Troubleshooting
- Reporting
- UX
- Incorrect problem definition
- Time management
- Ignoring other features.
- Focusing on today needs.
- Alerting
- Metrics
- Testing
- Deployment
- Code cleanup
- Latency
- No design review
- No customer review
- Concurrency
- Simple bugs
- Mutation
- Readability
- Naming
- Documentation (motivation)
- Audit
- Security
- Complex if-then logic
- Suspicious loop 'break'
- Negative logic
- Regexp - long input could cause issues.
- Nulls
- More than 3 arguments
- Line length
- Shared resources abuse
- Cache (think hard)
- Variable name length
- Commit message
- Squash if needed
- Build time change
- "Random" usage smell
- Open Close principle
- Code duplication
- OverComplexity (ex. Types where string is enough)
- UnderComplexity (ex. strings where type is required)
- Bulk test fail smell
- Backward compatibility
- Forward compatibility
- API no version
- Incorrect db type usage
- Adding indexes instead of search engine view
- Ignoring Failures
- Expecting feature would work
- Too verbose error handling
- Sensitive info in logs
- Missing "main" flow
- Feature toggle (if needed)
- Dependencies (collisions? new one?)
- Comments smell
- Dependent Teams
- Plural/Singular correctness
- Consistency with codebase conventions
- Revert implications
- Either simple MultiReturn or Single
- Time to understand code smell
[Update] Post AgentDenton comment: The way I use the list is just scan from time to time to ignite some neurons running in my rather dead brain, so when I get to code reviews, I use those few friendly code review neurons. Meaning I don't sit or use this list explicitly in code reviews, it's just the sitting on the back of my rather unorganized brain waiting for a code review to appear! ;)
I have yet to see a feature that was coded with the right requirements, if the developer did not question hard the requirements, it seems like always there is a slight change in requirement's if you think about the problem hard enough, if not reviewed properly, sometimes the requirements change altogether. If you didn't monitor, log, make sure you have tracing, back office in place, you will suffer in future. Did it effect reporting? if yes take care of it. Is the UX correct? Did you estimate the task correctly time wise. Are you focusing on today's need's too much, neglecting the future? it would cost you more than you think.
Clarification No "main" flow - I always like to see little mains so I can see the flow, (that is instead of a function that calls a function that calls a function). If someone manages to get a proper concise short "main" then everyone looking at his logic would know what the system does.
No chance I get to all above during a code review, that's why it's sitting on the back of the mind and not on the frontal cortex, I trust our unconscious brain to pop up the right thing at the right time, and if not, it's time to meditate to increase the chances of that happening. ;)
Top comments (3)
That is quite the list you've got there. It's not 100% clear to me how you use it in code review... Do you categorize comments into these?
If that's the case, I've adopted a similar style, but with fewer categories.
I use these as prefixes to my comment, so that it's rather easy to infer the sentiment behind the comment which follows.
Perhaps mine is too simple, but I've found it works for much of the time spent reviewing code :)
I like your expanded list, and I might adopt a few when the opportunity presents itself :)
Thanks I like that yours is categorized and concise. The way I use mine is more like as a general intuition snapshot reminder, I just scan it for a few seconds from time to time and when i get to code review some things from the list pop up and i use them. We could call it brain-pop-up-list to scan fro m time to time. ;)
Ah, I see now. It's not so much the comments itself as it is the things you are looking at when reviewing code.
In that case, this is quite handy. I do bear a lot of these in mind when reviewing code, but it is quite implicit in my mind already; many years of doing this sort of thing just does that :-)
But I'm actually about to introduce a code review process at a new company, and this would help explain to the rest what to look out for.
Great post!