What does Junior need to properly review Senior? Let’s take a look closer to this question and understand what it takes to become a good reviewer and submitter.

So, let’s understand what skills do you need to review a code and what skill do you need to create a pull request.

While I have been collecting my thoughts on this topic, someone already had done it and even had presented it on EuroPython conference. Unfortunately, I didn’t go to this conference, but there are slides in the Internet. And you defensively should watch it, because there is a bunch of useful information:

Some thoughts are very close to me because I’ve faced such problems in real life.

But the main point I would like to say is:

Do not be a perfectionist! There are only problems because of it!

Instead, your PR will be living for months and eventually will turn into technical debt. Do you want to solve conflicts after a few months, trying to remember what is going on in this code?

If the Pull Request is 90% good, then you can merge it.

What do you need to do a code review?

  • Don’t trust anyone! Review everything yourself.
    • “I was too lazy to add a check”, “I was so tired that I forgot to fix it” – all these things you can miss if you trust the person and didn’t check the PR properly. Don’t be a clerk, who puts the stamps on the documents without reading them.
  • Don’t be smart! A code should have very good readability. 
    • Don’t forget the joke: “A good code is like a good joke – it doesn’t need an explanation”
  • Be patient.
  • Be objective.
    • Use “a comment is skipped in this method” instead of “you forgot to put a comment here”.
  • Ask questions instead of leaving answers
    • “What do you think about …?”, “Will it make sense if …?”, make suggestions: “It might be easier if …..”
  • Leave clear comments
    • Stick only to your opinion, tell “how and why”, attach links to documentation, articles, and examples. “This line of code makes me sad” is not a good comment.
  • Don’t just criticize. Encourage good ideas and solutions.
    • It might be a good idea to leave at least 2-3 positive comments in each PR.
  • Don’t be a perfectionist.
    • Prioritize what’s important to you. If everything is 90% good – that’s enough.
  • First, make the most critical comments on the architecture and design, and only then on weak variable names and grammatical errors.
  • Try to make a review 24-48 hours after creation.

What do you need to be a good submitter?

Start with ….

But seriously, it is…..

  • You are the main reviewer.
    • Review your code as carefully as if you were reviewing someone else’s code. Try to predict problem areas.
  • Check and test your code before making a Pull Request, and don’t force others to test your code for you.
    • I worked with a programmer who didn’t run his code at all. He explained that he gets paid for writing code, not for checking and testing it. As you know, not many people liked him…
  • Make a checklist for yourself before creating a Pull Request. Did you check all the items from it or did you forget anything? It’s enough to have some small checks like “Have I deleted all lines of commented out and unnecessary code?” or “Have I checked there are no unused imports and variables?”.
  • Make a checklist with more global checks: “Is it easy to maintain this code?”, “is my code have a resistant to crash?”, “is there any security issues in my code?” (Yes, there are still those who send passwords in plaintext using a get request)
  • Don’t touch your Pull Request too hard during the review process.
  • If you develop a big feature and want others to see the process, open Pull Request when the code is written for 30-50%. Don’t be afraid that the code isn’t finished yet. Your colleagues can tell you about architecture problems and suggest the necessary design patterns.
  • Try to make small Pull Requests so it is convenient to do code reviews.
  • Check the code using automation tools.
  • Use pre-commit hooks.
  • Write unit tests. Always. Don’t be lazy. They can point to problems immediately.
  • Always respond to comments in Pull Request. Even if they are ” fixed” comments. If you can’t fix something, or you don’t need a fix, write it and explain why. If you decided something with a colleague over lunch, write this decision in the Pull Request so that others can see it too.
  • If you pushed something, let your colleagues know.
  • If you think your solution is better than the one suggested, fight for it!
  • Don’t take comments personally. Think of this as the key to your professional growth.

In the end

Code review is a very important part of the team ‘s life. It is impossible to downplay the significance of this process. This makes a team stronger, a code better and cleaner, and gives growth not only to beginners but also to those who are already experienced. A person who can do a good code review is worth his weight in gold. And always remember that it was a time when none of us reviewed a code or created a Pull Requests, so be patient with those who haven’t done it yet, and the world will get a little better.

Do you apply some of these concepts in your team? Please, share your experience in the comments below!

Categories: Programming


Leave a Reply

Your email address will not be published.