A good pull request: what is it and how to do it?

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 available. And you defensively should check it, because it contains 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, you can merge it.

What do you need to do a code review?

First of all,

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 it: “A good code is like a good joke – it doesn’t need an explanation”.

Be patient

It's not helping if someone is rude or aggressive in comments of a PR.

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 .... What do you think about it?”

Leave clear comments

Stick only to your opinion, tell “how and why”, attach links to documentation, articles, and examples. “This line of the code makes me sad” is not a good comment at all.

Don’t just criticize, encourage good ideas and solutions instead

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 to merge.

First, make the most critical comments

On the first place you need to focus on the architecture and design, and only then on weak variable names and grammatical errors. Leave comments like about wrong spaces on the very last round of review or consider to use automation.

Try to make a review 24-48 hours after creation

I remember PRs which could leave 7 months. Everything was fine, people just didn't have time to review it.

Conclusion

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.

If you would like to become a better submitter, please take a look at the article “A good pull request: how to become a better submitter?”

Do you apply some of these concepts in your team? Please, share your experience with me!