About code review
For me, ideally, there should be up to 5 things we check in a pull request, namely:
1. Architecture and structural issues. Examples are: Why did we add that abstraction here? What is the pattern here? How has it changed the architecture of the underlying module?
2. Naming issues. Example: Is this name clear enough to describe what is going on here?
3. Logic of code. Example: Does it solve the right use case?
4. Clarity of code. If you don't understand what is going on here within a 5-second interval, then it's not simple enough and maybe it should be rewritten.
5. Assessing potential risks of the code. Example: What if we make a lot of requests here? Will it slow down the entire use case for users? Is it important?
But for every other project the set of such things can be different.
Still, if possible, some things should be kept out of the code review process, namely:
- Conventions and code style checks should be automated.
- Tests should guarantee a certain degree of reliability.
- Personal taste in code writing shouldn't be argued about.
