Pull Requests Etiquette
June 25, 2019
It is showed that frequency of deployment is at the heart of delivery performance. And pull requests are usually a mandatory component in this process. Many little things can make a pull request slower and more painful that it should be. Bellow is a list of problems I see common in PR’s and how to avoid them.
Maybe the biggest problems with PRs is the PR paralysis. When someone stay blocked and cannot proceed working until someone else takes a look in the PR.And the review cycle can involve more than one of this rounds in a single PR.
My suggestions is: do not wait for approval after the first round of comments. Reviewers should comment on everything they can in the first round. If the reviewer really needs you to make a change before proceeding he/she should use the “require changes” option in Github.
We want to avoid the typical unprofessional PR:
- reviewer adds a comment
- dev fixes or reply
- reviewer goes back to the pr and find another one
- dev fixes or reply
And so on and so on…
Once you fixed the points or “settled the case” with your reply you should merge without requiring the green badge of approval by the reviewer. The idea is that everyone in the team is given trust to merge if they know the reviewer will be satisfied with the answer.
Too many pull requests
Reviews flood in order to keep process may also be a problem. Not everything should be reviewed. If some change is small enough or inconsequential enough, use your good sense and just merge it. Here again we deposit trust in our team members and left their good judgment to rule.
Involve the minimum amount responsible of people
We should not look for team consensus in a PR. First of all it Is about implementing requirements in a correct way. Second, is about non-functional requirements, readability, etc.
Involve not less than the minimum
On the other hand, another problem that might arise with PRs is the merge without consent. If you are doing a consequential change in the future of the software you should involve senior team players. At the same time include only the minimum amount of people that is necessary to get the thing done responsibly. You gut will give you the sense.
Keep it objective
If you are a reviewer be careful not to introduce topics that are totally unrelated with the PR at hand. You can reach-out the person, create tickets, but the PR should not fix all the problems of the world.
In the end of the day, is your responsibility as the PR creator to get the task to done. If the reviewers are taking too long to do their job is your responsibility to go after them and push them to finish reviewing. Don’t accept that you have to remain blocked for long. If your team is healthy you should unblock each other fast. Either by they being fastly prompt or you being proactive and remembering them to do their jobs.