How I (do) Code Review

Este post está em inglês porque compartilho ele com meus colegas de trabalho que não leem em português.

When I am reviewing your code, I love to learn and help you with proposals that I believe that should improve it. When you read my reviews, keep in mind that:

The code is yours. You can disagree with me, and I am completely fine with that.
My comments are about your code and not about you.
When I ask something, I do it to understand the full context of your work to write better proposals.
I am dumb and make mistakes (a lot of them). I tried my best and failed? Just point me the problem and I will fix it.
I am not a native English speaker, and code review is mostly made with limited text messages. If you don’t understand or feel bad reading something that I wrote, it is my fault and I apologize for that in advance. I probably wrote something in a bad way and I would be grateful if you point me out these things. It will help me to improve.

I have four types of comments:

Recommendation / Suggestions (see more about this below)
Questions. I can ask questions to:
- Get additional information about the context (Is it a critical bug? Something urgent? Worth a bigger refactoring?);
- Understand the issue that you are solving.
Code Suggestion. For small things that I can edit directly during the review process.
💀 Dead code is dead. We use VCS/git, and we don’t need to commit commented code 🙂

When I am reviewing the code, I have three levels of recommendations (and an off-by-one level 😛):

Ignore: I see something that should be improved, but I know that it would lead to a massive refactoring that it’s not worth it. Sometime it will generate a proposal for the team that leads to an RFC or a card on the board. Occasionally, I just ignore and keep on 🙂
Format: Things that Black & Ruff should fix automatically for us. Regularly, I use the 💄 emoji to point out these suggestions.
Style: Coding style. Things that go beyond code formatting but are still something that should improve, at least, code readability. Those recommendations are completely personal, and It is not fair to force it into the team. I can also use the 💄 emoji in these comments. Some things that I (emphasis) (dis)like:
- I prefer return-early pattern;
- I don’t like to use dict() for data structures;
- I “hate” elif‘s and else‘s (I am a OOP-guy and that polymorphism-thing 🙂);
- I am scared of if/elif with no else;
- I don’t like to return None as an exceptional return (unless I am coding in C or Go);
- I don’t like to use flag variables to control the flow of the code.
- [Some other things that I will add here when I remember].
Fix: these recommendation points for real problems in the code.