Improving Pull Requests

When working in a team, it is always good to have your code peer reviewed before merging to the main branch. It is always a space for improvements and learning and not about “you”. I have caught multiple issues and prevented unintended behaviour from being shipped to production. I also got valuable feedback from my peers.

In this blog, I discuss the knowledge I gained from books such as Clean Code (by Robert Martin) and feedback from my peers and implement them in the real world.

The missteps

Initially, the pull requests I would create would be long, thus making them challenging to review. It would contain the new feature in its entirety. In my case - a database entity, service, API route, corresponding integration and unit tests validate the business logic.

A single PR could be huge when there are multiple things to review and not because of the number of files involved.

Build from the ground up

Understanding how one can create compact PR comes only after making many larger PRs. With experience, I understood how to slice the code and delivery the feature, which gradually builds up, and the final PR would tie it neatly.

I first try to understand from the external consumer perspective what is needed. Because without understanding the user, I cannot build things from the ground up. I list items from the end customer pov to the moving parts of the code. Then start working on writing individual parts of code and then try to tie them up.

Handling the pitfalls

Nowadays, we focus on continuous deployment, which could mean deploying to production often. To avoid my unfinished code bringing the production environment down, I would do one of the following:

1. Feature flags
2. Not initialising the service during the boot
3. Mounting the API route at the very end

Feature flags are powerful, and so much has already been discussed on the internet. For the other points, my thought process would be to avoid initialising the API or service. If I mount the API route first, it prevents me from modifying the contract. So I would always create the API for the external consumer at the very end. If the services are initialised at the start of the server before everything is in place, the server might go down in the worst case or perform unintended behaviour at best.

Creating the PR

As I have sliced the code and handled the pitfalls, I have to create the PR to be comfortable for the reader’s eyes. I write up the description and add images if they are needed. Sometimes I have added a gif/video which shows the feature.

If there’s any manual test report or a CSV file generated by the endpoint, they will be available for the reader. I take a brief break and then review the things I’ve added in the description of my PR. If it needs improvement, I make them or publish my PR for review.