Why you should always open a pull request when working in a team

Be a positive team player

Before starting my position as a software engineer at Nested I had a very limited experience of working in a team of developers. Most of my personal improvement efforts were focused on acquiring the skills required to write good code and manage a small in-house product team. What I’ve learned at Nested is something that’s hard to come across in tutorials or courses, mainly — what it means to be a positive team player.

One can be a positive team player in many different ways. However, this time I will focus on pull requests and why you should always open one if you have at least one other person writing code with you. To understand the problem, we must look at how my journey began.

In need of a trustworthy expert

I always liked coding, but never took it seriously. My father was a successful businessman and I was always eager to follow in his footsteps. However, things changed when a customer relationship management (CRM) system developed for my family business was malfunctioning and the company that built it started to charge increasingly higher fees for simple modifications, like changing the label of a column in an order list. The system was increasingly important for the business, because it allowed us to manage to increase our turnaround significantly with the same amount of people. The system was developed by a small local IT company. Nobody in my family knew how to code and we weren’t about to trust other companies after the painful first experience. Since I always excelled at learning new things and was always drawn to programming I decided that I will learn how to fix and improve the system myself.

I am the expert

And so I did. I started to devour online courses and dabble with our CRM system to learn how things work and what can be improved. It didn’t take me long to see a lot of copy-pasted modules with lines and lines of nested if statements and a bunch of other bad practices, which I learned about from the online courses. Fast-forward 6 months and I was overseeing a team of freelancers and an in-house developer working on an IT project, which helped our clients to order our products online.

Desire to learn more

After a quick initial learning period I plateaued. My family business was doing good, but not great. We were growing at 20%+ rate per year, but we were still small. IT was no longer the bottleneck and so the resources had to be focused on other departments. There was no longer a need for a strong push from IT. So I decided to leave the family business to continue improving my skills as a developer.

Learning continues

When I joined Nested we started migrating from the monolithic Rails app to React with GraphQL and Elixir as the backend. I was eager to build things and so I continued to work as I was used to — building and shipping things straight to master.

Soon enough, someone brought pull requests (PRs) up and we had a discussion where we decided to open a PR whenever we want to push to master (at that time it was usually a web page rebuild using React). I felt that this will slow me down, because I was often working after hours and on weekends when no one was around to review my PRs. However, my desire to contribute was triumphed by my desire to learn from my peers and so I started opening PRs whenever I wanted to merge something to master. My initial PRs were huge (1000+ lines changed), because I was still learning a lot of new things and refactoring as I go. Often there was more than one page involved. This didn’t help when someone wanted to review them.

And so another discussion followed about keeping our PRs small so that it would be easier for the person reviewing it to understand the PR as a whole. Small, focused PRs have even more benefits than just opening PRs in general.

Being a positive team player means opening those PRs

This is great for your team, because your peers will:

• be able to ask questions;
• have a chance to review your code and submit their feedback;
• be able to keep up to speed with the changes;
• have an opportunity to learn small tips and tricks which you may take for granted.

While this is good for your team, this is also great for you personally as well, because this practice will encourage you:

• to keep your changes to the minimum required;
• to be on point (one thing per PR);
• understand why you are making the change (you will need to write a good PR description);
• to improve your git history.

Excuses to avoid opening a PR

Often I would make a small change or bug fix and push it straight to master, because I couldn’t wait a day for my PR to be reviewed. Luckily, these excuses were eliminated in team discussions. Below are all of the excuses I had myself.

Small change

All I want to do is reduce left padding from 20px to 10px, surely I don’t need a PR for this…

The problem with making exceptions is that you have to draw a line somewhere — how many lines of code does a change need to affect for it to be considered big? Are you sure that no one will need to know about this?

Personally, I’ve seen even the smallest of the changes causing big problems. Having a second pair of eyes look at your code or simply taking that half a day break while waiting for an approval can be a difference between a buggy code and a beautiful code. If you work on a large project where stakes are high mistakes often cost more than the value gained by shipping some code x hours earlier.

Another reason why this excuse is negatively impacting your team is that it takes away control from your peers. Adding reviewers makes everyone feel included and they can decide for themselves if they want to keep doing this or not.

Adding reviewers

I can’t be bothered to always add everyone who may or may not be interested…

You can add teams or make use of CODEOWNERS file.

I don’t make mistakes

I’ve done this a lot, I won’t make any mistakes…

or

It’s a simple change…

Again, this may be true in the majority of the cases, but sometimes (especially if you are not senior) you just don’t know what you don’t know and you can’t see the problems that your change could be causing.

This may also send a subconscious message that you are better than others and you don’t need their help. This can alienate you from your team and cause unintended bad feelings. Asking for feedback is always the best way forwards and allows others to help you to improve while giving them the opportunity to learn from you at the same time.

Long PR review times

It takes ages before someone reviews my PR…

Well, that’s not a problem with opening a PR. It’s a problem with reviews. You need to agree with your peers to prioritise those PR reviews. Also, make sure that your PRs are easy to review.

Exceptions to the rule

Exceptions to this rule obviously exist, but I can’t list them for that very same reason — they are exceptions. You’ll know when you’ll encounter one. If you feel that there are cases when you need to push straight to master then discuss this with your team prior or, if necessary, after the fact.

Conclusions

I feel that following this rule made our team at Nested much better. I had the opportunity to get and give feedback on many, many occasions which undoubtedly improved my coding habits a lot. It also feels great being asked to review PRs of my peers.

Think that you have a great reason why you shouldn’t always open a PR when making changes to your main codebase? Or maybe I missed a great reason why everyone should do this? Please let me know in the comments!

Want to work with me? Great news, we are hiring!