If you have to review a Pull (or Merge) request in an unfamiliar C# Code bases, what will you check? Here is "my method" (or check list) to review a C# codes base without knowing what it is doing.
As we don't know much about what the code is doing, we can only review on the "best pratice" aspect:
- where are the tests which would cover all the changes? justify if the changes are not unit-testable (yes, it can happen)
- challenge if there are unjustified
try / catch - challenge if "Generic" or "Reflection" are used (over-engineering smell)
- challenge if "Inheritance" is used? (over-engineering, bad design smell)
- challenge if more interface, abstract class are added? (over-abstraction smell)
- challenge if Static method / class is added: Bad if the static methods change the application state (risk of non-testable code)
- logging best practices
- not having enough info log messages for important "if" branch
- log contains sensitive information?
- high perf logging?
- if a "lock" or a "transaction" are openend => check how are they managed / shared in the code?
- if something is cached => what is the cache invalidation policy? take a look to this process.
- if a new Task/thread is created => check how the errors (comming from them) are handled?