Clear Pull Requests

I have been working as a remote software engineer for various teams from around the globe. I have used an array of different project management tools over my journey, all aiming to ensure that the project is efficient, maintainable, scalable, well-structured, readable, modular, and well-documented.

Let’s focus on how to ensure and maintain software quality. More precisely, I will focus on the pull requests (PRs) and code reviews. Both of which are very much needed to create high-quality software.

(Not familiar with GitHub flow? If so, I recommend you read this before moving forward.)*

Here you go, my guidelines for meaningful PRs:

1. It takes time but it’s worth it

Opening a pull request (PR) and getting it reviewed is an important stage before the commits do their magic to the dev/live branch. Pushing straight to the master is always a serious crime! PRs assist reviewers in knowing what has been done for each ticket. So remember: always link your PR with a ticket and vice-versa.

PRs should give your fellow developers the big picture of the feature. It might seem tardy to describe the feature and its implementation but doing it will greatly help the reviewer to understand the feature. The time you consumed on this process will pay off as the review will be a lot faster.

2. Succinct title and detailed description

The importance of the title shouldn’t be overlooked. One should be able to tell precisely what is done in the PR. If you can’t write a concise title that implies there are too many things covered in the PR, and that is a mistake (of which I will discuss more below).

Also, keep [WIP] (work in progress) indicator in the PR’s title if you’re not done with it. It will keep the reviewers from wasting their precious time on review too early.

In the description, write as many details as you want to, such as the solution you’ve followed and the reason for that, any issue that you came across, any tech-debts involved, functionality, screenshots, and videos to support your PR. Don’t forget to mention any operations done outside of the codebase (e.g., configuration updates).

Organisations should prepare templates containing placeholders, e.g., described here.

3. Don’t create huge PRs

What will happen if you are given a PR of 5k loc changes? Maybe 5 more cups of coffee that day…

Always try to keep PRs covering only one single feature, bug fix, or enhancement. When the size of the PR is huge, it becomes difficult for the reviewer to get the big picture, and the review process might take ages.

Having a very creative day or a big feature in hand can lead you to a huge PR. In that case, split your Feature into smaller subtasks and open a PR for each of them. All should be logically connected so that they can be merged into the Feature PR.

4. Support your PR with tests

For every change made in the PR, functionality tests should be written properly. This is also evidence for your feature to be working in all the corner cases and will help to fix future regressions.

5. Configure automated test stage in CI

Running the tests on every commit should be automated through handy CI tools, such as GitLabCI, TravisCI, CircleCI, etc.

Integrate coding linters and tests’ coverage tools in the build process, too, so that the pipelines indicate the violations way before the reviewers do so.

6. Avoid non-related changes and refactoring

Follow the single principle rule, and don’t make unnecessary changes in a PR, not even formatting and lint fixes. If you really want to fix multiple things, keep them in dedicated PRs.

Changes in unrelated places in a PR are not the only thing that can make a developer lose their focus. For example, indentations should be done in a separate branch too.

indentations_o.png

7. Provide solution references, comments, and evidence

Use comments to help reviewers. Although well-documented code can be considered the best standard, sometimes the context might need your explanation. Write explanations in PR effectively, provide references where necessary to the solution you’ve adopted. All this will save your and the reviewer’s time.

If your changes can be shown via an image or GIF, go for it. This can help reviewers know what exactly the changes will look like or how they perform in the application.

8. Integrations

Integrate your primary communication channels (e.g., Slack) with your git-based repository hosting service (e.g., GitHub or GitLab) so the team stays informed about your work.

9. Rebase vs Merge vs Squash

This can be a long discussion with strong reasoning for each option, but personally, I suggest learning proper rebasing flow to take the entire feature branch to the top of the base branch and have commits working effectively there.

In addition, squashing should be avoided as this cleans up the commit history and can overtake the contributions of fellow team members’ contributions in the feature branch. You can read more about the topic here.

Most important takeaways

  • Never ever push directly to live branch
  • Get your team to review your work
  • Keep your work’s PRs clear and concise
  • PRs should be properly documented
  • Write tests for your changes
  • Have automated build and release stages for your PRs

Thanks for reading. I hope this helped you to tune your development process even better.

For more help book a meeting with us to discuss how our software development expertise could help your business grow.

Muhammad Arslan

Contact us

We are here to help your company's technology bloom.
So do not hesitate to contact us in any matters!

Read more insight in our blog