If you have to review a Pull Request (or Merge Request) in an unfamiliar C# codebase, what should you check? Here is my checklist for reviewing a C# codebase without fully knowing what it does:
Since we don’t know much about what the code is doing, the review should focus on best practices:
- Are there tests that cover all the changes? If not, is there a justified reason why the changes are not unit-testable? (Yes, that can happen.)
- Challenge any unjustified
try/catchblocks. - Challenge the use of Generics or Reflection (often a sign of over-engineering).
- Challenge the use of Inheritance (may indicate poor design or over-engineering).
- Challenge the addition of extra interfaces or abstract classes (potential over-abstraction).
- Review any static methods or classes: these are problematic if they modify application state (risk of untestable code).
- Logging best practices:
- Are important
ifbranches missing log messages? - Do logs contain sensitive information?
- Is high-performance logging considered?
- Are important
- If a
lockor a transaction is opened → check how it’s managed and shared in the code. - If something is cached → what’s the cache invalidation policy? Review how this process is handled.
- If a new
Task/thread is created → check how errors coming from them are handled.