13 Jan 2020
- by Muhammad Arslan
Lucid 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 discuss project management more generally some other time and now 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 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
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 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 description write as many details as you want to, such as solution you’ve followed and 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.
In pic you can see an example of a pull request I left for my colleague.
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 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.
Follow 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.
Use comments to help reviewers. Although a well-documented code can be considered as the best standard but still sometimes the context might need your explanation. Write explanations in PR in an effective manner, provide references, where necessary, to the solution you’ve adopted. All this will save your and reviewer’s time.
If your changes can be shown via image or GIF go for it. This can help reviewers to know what exactly the changes will look like or how they perform in the application.
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 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. Hope this helped you to tune your development process even better.
If you are interested to hear how me and others of Montel’s seasoned software developers and DevOps experts could help you create great software, please don’t hesitate to contact Mikko “tuba” Tuominen, +358 400 636636 / firstname.lastname@example.org.
I’m also happy to hear your thoughts about the post so don’t hesitate to leave your comments below.