Tomer Ben DavidCode 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. ;)
