A pull request is a good way to propose and collaborate on changes to a repository. Writing good pull requests and having an effective workflow will increase a team’s productivity and minimize frustration.
Expected benefits of a good PR:
- It tends to be reviewed quickly;
- It reduces bug introduction into codebase;
- It facilitates new developers onboarding;
- It does not block other developers;
- It speeds up the code review process and consequently the product development;
1. Before Opening the Pull Request
The checklist below aims to reduce the noise among the author and their reviewers.
- Name the branch accordingly to the team’s convention.
- Explicit whether the Pull Request is a draft (or WIP).
- Go through the changed code looking for typos, irrelevant comments, debugging pieces of code, trailing whitespaces, etc.
- Combine multiple commits into one to make it look like a story (or a task).
- Rebase your branch onto master and upstream branch.
2. Make Smaller Pull Requests
Making smaller pull requests is the #1 way to speed up your review time. Because they are so small, it’s easier for the reviewers to understand the context and reason with the logic.
Small PRs are :
- Reviewed more thoroughly and quickly.
- Easier to merge.
- Less wasted work if rejected.
Here are some ideas to make it easier to make small PRs:
- Extract refactoring to a separate PR.
- Split big features into parts (even if they wouldn’t be user-facing).
- Make
git add --patchyour friend! ( it allows you to go through your code one more time before committing).
3. Clear Title and Description
When creating a pull request you should care about the title and the description as a way to provide an overview of why the work is taking place with links to the JIRA ticket and any other relevant services.
Start the description by stating the purpose of your work. Something in the lines of :
This is a spike to explore…This simplifies the display of…This fixes handling of…
Do not assume full familiarity with the story or work that has been done. Imagine that the code reviewer is arriving in your team today without knowing what is going on, and even so, they should be able to understand the changes. Remember that anyone in the company could be reading this Pull Request, so the content and tone may inform people other than those taking part, now or later.
Mention colleagues or teams that you specifically want to involve in the discussion, and mention why. (“/cc @person for clarification on this logic. @ops, any concerns with this approach?”).
For client-side changes, add some screenshots or gifs for changes or animations.
Similarly, a useful summary title (instead of just the issue key) makes it clear to the reviewer what’s being solved at a high level, which takes off one extra step for the reviewer.
Keep the title short, but expressive enough to signal what the reviewer can expect.
3. Code review
As a reviewer:
- Prioritize PR review.
- Make an effort to read the description and understand how the feature works.
- Use positive language as opposed to neutral. (If the content is neutral, we assume the tone is negative.)
- Write clear comments. Describe the issue, why you don’t agree (mistakes, errors, violations against conventions, performance risk, security issues, etc.) , and propose other solutions to simplify and improve the code. Don’t be afraid to ask questions.
- Write good and bad comments. If you like a piece of code don’t hesitate to let the developer know!
- Use a checklist to try to catch any errors, bad practices.
- Pull the code locally to test the acceptance criteria of the feature and the performance before approving.
As a developer:
- Two reviewers are better than one! don’t merge your code until at least 2 reviewers approve.
- Do not change the code while reviewing.
- Answer all comments and explain your point of view.
- A common axiom is “Don’t take it personally. The object under review is of the code, not you.” Assume the best intention from the reviewer’s comments and be aware of how hard it is to convey emotion online. If a review seems aggressive or angry or otherwise personal, consider if it is intended to be read that way and ask the person for clarification of intent, in person if possible.
Finally,
To sum up, keep in mind that a good pull request is reviewed fast, which means merged fast!
Top comments (1)
Very good.