This is the first of a 3 part blogpost series about Pull Requests. I am writing this so I can give you my personal view on each side of the Pull Request saga, something that I will explain to every member joining my team. It contains about the following 3 parts:

• Ethics for the Author (This post);
• Ethics for the Reviewer;
• Improving the Pull Requests process;

It is annoying right, not commiting on the master|main branch and that you should work on a branch, create a Pull Request that people should review before it can merged into the master|main branch. But it all has a good reason, but this process just takes a little bit more time to get your code merged into the master|main branch. We do this not because it is cool or a fun thing to do, but we do this to either keep the current quality for the code and/or to improve it when you add new functionality (tests).

But lets not go into details on that, let us focus on what to actually do with a Pull Request and how we should act on it. With a Pull Request, you can either be the Author (Creating a Pull Request, because you have written code that you wanted to be merged) or a reviewer (You are the extra set of eyes to take a look at it). This blogpost is about being the Author.

Don’t make it personal

We start with the most important one: don’t make or take things personal towards others. We are all people doing the best we can, (most) probably working for the same company and thus having the same goal: Doing awesome work for an awesome software project for an awesome goal/service. So don’t make any comments to others like “This is way over your head”, “You are stupid, it is like …” or “Just approve it dumbsh*t” etc. I know, I exaggerate a bit but you understand what I mean right? If a reviewer is not understanding the pr/change/etc, spend some extra time to help him/her by explaining it so (s)he will understand it. If you are working for the same company, have a (zoom)call or drop by, or have a chat personally to explain it so everyone is on the “same level”.

You should never start or be part of a flaming war, this will help nobody and will only cause severe atmosphere issues in your team/community with either people leaving or just stopping to work at the project.

Don’t fall in love with your code

You probably did all the best you can do on this piece of code you have written. You might think it is your best work, which could be, but keep in mind that a 1000 roads leads to Rome. This is also the case with code. You implemented 1 way of doing something, while there might be different ways to “get it done”. Just because you think it is your best code “evar”, it does not mean that it is the actual best way and can not be improved at all. You are most probably wrong. You wrote the code with the best intention, based on your knowledge and experience you have on the given subject. A reviewer might have more or different experience and knowledge about the subject and is able to give new insights for your code to improve it. Be open to these suggestions that the reviewers are giving and where needed, have a direct contact/chat with that person to explain/discuss it. When you are open to these comments, you are able to learn from it and will help your (future) coding practices.

This also applies when someone declines your Pull Request. Don’t go mad or insult the reviewer. (s)he is doing just his/her job and lets be honest, you asked for their input! But when the Pull Request is declined, have a chat/talk with the reviewer and discuss it properly. Try to understand the reasoning and try to resolve it correctly. If after the discussion the conclusion was that the reviewer was right, no worries and try to apply the reviewer comments and try again. If after the discussion the reviewer was wrong, no worries again and recreate the Pull Request. But when you have recreated the Pull Request, add a small comment with a (short) summary about the earlier decline so it is clear for everyone what the result is.

Reviewer is not always right

Just because a reviewer is making a comment about something, doesn’t mean (s)he is automatically right. (s)he does this with the information (s)he has at that moment and makes a comment about it. (s)he does not know anything about the past while you were developing the functionality that have lead to what you have right now. And maybe your Pull Request was the nth Pull Request that the reviewer was reviewing, so maybe (s)he was mixing things/knowledge with other Pull Requests? But in any case, talk/chat directly with the reviewer to sort it out and when you concluded things together, update the Pull Request with a comment with this information so everyone knows about it.

Do make sure the why

And this is something you should start with, create a good and understanding description of what the purpose of the Pull Request is. Provide for example a link to the userstore/issue with some information. When you have Jira and Bitbucket for example, you can create a branch in a git repository that is automatically linked to the userstore/issue so even from the name of the branch it is automatically clear on which issue you are working on.

When you are working on a public available repository then you only have access to the “issues” part of f.e. Github/Gitlab. Check if there is something like a contribution section in the readme or maybe there is a specific document for it. Most of them explains what is needed to be done to be a contributor to the repository, like creating an issue with a proper description and with reproducing steps (In case of an issue). Make sure that it is all clear for everyone and then create a Pull Request and make sure to link it to your earlier created issue, or the issue that you picked up to provide a fix.

So really make sure before any reviewer starts doing reviewing that for everyone it is clear why this Pull Request is created and what it does solve.

Do make your own comments

There is nothing wrong with making comments on your own Pull Request. And with this, I mean that you can make a comment on why you have taken this approach or maybe add some background information that would help the reviewer to properly evaluate the Pull Request. While you were developing or working on the change, it might be that you have created several different attempts to “get it done” before you went to the solution you have right now. With making a comment as an Author on your own Pull Request, you can describe these tasks and it helps the reviewer understanding why you took “this” approach so they won’t have to ask you.

Summary

There is also maybe the downside of doing Pull Requests, you can create 1 small Pull Request that took many hours or maybe days to complete, because you have tried different things which didn’t work. A reviewer will not see this, because (s)he only sees the end result and not the things that have lead to this Pull Request.

And that is why I suggest to focus on the earlier mentioned ethics, make sure that the reviewer knows why the Pull Reqiest is created, be open for suggestions from the comments from reviewers and comment on your own Pull Requests on the areas you will know that either helps the reviewer or you will know for sure someone will comment on it. This will make the whole Pull Request procedure a lot easer for everyone.

One of the most important aspects of working with Pull Requests is proper communication between the author and the reviewer(s). Be open to each other and respect each other opinions and keep in mind that the focus is on keeping the quality and not someones ego. Don’t use the comment section as a chat application but just contact the reviewer directly if there seems be a discussion going on. And again, don’t make things personal. It helps nobody if you make things personal and it will only work in the opposite way.

What do you think what are the important ethics for the author that I missed. Please add them in the comments below and I will update this blogpost.