Code Review Checklist (Brain Backup/Export)

Code Review Checklist:

1. Logging
2. Monitoring
3. Scale
4. Traceability
5. Troubleshooting
6. Reporting
7. UX
8. Incorrect problem definition
9. Time management
10. Ignoring other features.
11. Focusing on today needs.
12. Alerting
13. Metrics
14. Testing
15. Deployment
16. Code cleanup
17. Latency
18. No design review
19. No customer review
20. Concurrency
21. Simple bugs
22. Mutation
23. Readability
24. Naming
25. Documentation (motivation)
26. Audit
27. Security
28. Complex if-then logic
29. Suspicious loop 'break'
30. Negative logic
31. Regexp - long input could cause issues.
32. Nulls
33. More than 3 arguments
34. Line length
35. Shared resources abuse
36. Cache (think hard)
37. Variable name length
38. Commit message
39. Squash if needed
40. Build time change
41. "Random" usage smell
42. Open Close principle
43. Code duplication
44. OverComplexity (ex. Types where string is enough)
45. UnderComplexity (ex. strings where type is required)
46. Bulk test fail smell
47. Backward compatibility
48. Forward compatibility
49. API no version
50. Incorrect db type usage
51. Adding indexes instead of search engine view
52. Ignoring Failures
53. Expecting feature would work
54. Too verbose error handling
55. Sensitive info in logs
56. Missing "main" flow
57. Feature toggle (if needed)
58. Dependencies (collisions? new one?)
59. Comments smell
60. Dependent Teams
61. Plural/Singular correctness
62. Consistency with codebase conventions
63. Revert implications
64. Either simple MultiReturn or Single
65. 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. ;)

Comments

[Tinus Smit]

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.

• Convention
  • Naming conventions, patterns, process, etc.
• Error
  • Blatant error in code / logic / understanding.
• Logic
  • Purpose of the code is not clear from simply reading it.
• Simplify
  • Opportunity to refactor code into a method or replace code with framework method.

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 :)

[Tomer Ben David]

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. ;)

[Tinus Smit]

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!