How I do code review

Understanding change

When I review PR I want to understand it. I want to have full context why this change is made and what each line is doing. This allows me to provide actual value rather than blindly stamping with "LGTM" comment.

Understanding other people's code is hard. Especially when I'm not in context of a project. When I review a diff I spend time getting into context. Every time there are new changes to the same PR I have to spend time again.

When PR is large and doing multiple things (refactoring, fixing bug, adding new logic) time to get into context and understand every change may take hours. I'm happy to spend time reviewing code but it's fair to ask author to help simplify review process.

The best way to ensure review is fast and provide best feedback possible is PR should do only one thing. It means breaking large PRs to smaller parts that usually fall into one of 2 categories:

• Refactoring without logic change. Often large and without changes in tests. This allows me to focus if code moved properly and I don't need to suggest changes in logic.
• Logic change. Small and include tests. I can truly focus on a change, find issues and suggest improvements.

Commenting

I sort my review comments into three main categories:

1. Critical Bugs

   Rarely, I find changes that cause big bugs or performance problems that the author didn't intend. I will point these out in a comment and block the PR. It's important to fix these issues, and the team usually values this.

2. Useful Suggestions

   These comments are about suggesting changes like refactoring, performance optimization, or choosing better names. I make sure these suggestions are helpful, giving clear reasons and other options. For example, instead of just saying I don't like a certain name, I explain why and offer other name choices. I don't block the PR just for these suggestions. I trust my teammates to look at my feedback and address if they think it's right. We talk openly, and I expect explanations if there's any disagreement, but I don't force changes.

3. Nitpicking

   This group includes small, personal preferences that could start unneeded arguments, like small differences in naming or coding style. I usually approve PRs that only have these small issues and I don't require explanation why they are not addressed. To make it clear I mark them with "Nit".

This approach helps to keep a good balance in our review process and supports a team environment built on trust and helpful feedback.