Pull Requests: The Lodestones of Collaboration
My intent in this article is to share a few tips that we’ve found valuable for incorporating Pull Requests into our workflow. First, though, I’ll spend just a tiny bit of time on the basics for those who aren’t familiar with the concept.
At a basic level, a Pull Request could be thought of as a wrapper for submitting contributions to a development project that is using a distributed version control system. The information incorporated in a Pull Request includes, at the highest level, the source repo & branch, the destination repo & branch and meta information related to the request. The source repo may be either a clone, or a fork, of the destination repo. The workflow data associated with each of these and the support incorporated within the Pull Request mechanism allows team members to review the proposed changes, discuss alternatives and even push additional commits (if required).
At GripQA, we use Git to manage our sources with GitHub as our hosting solution. We have a mix of public and private repos. These notes should apply to both equally. So far, we’ve tended to stick with the Shared Repository model for our approach to collaboration. It may be worth mentioning that we consider the “Pull Requests” to be a method of the workflow, not necessarily a feature of the version control system. Other code management tools, like Bitbucket also support Pull Request functionality.
The following are in no particular order, but they’ve helped us make more effective use of this powerful tool for efficient code collaboration:
- While the Fork and Pull model works great for the well known open source projects, we find that the Shared Repository model is better suited for our relatively small teams of senior software engineers that collaborate closely on a daily basis. This will certainly change when some of our public repos begin receiving proposed changes from outside the immediate team.
- We strongly encourage teams to use all available tools to ensure that the changes included in the Pull Request are sound. This should include defect analysis, entropy analysis and appropriate static code analysis.
- This should go without saying, but it is the Pull Request submitter’s absolute responsibility to ensure that the code builds and that all tests, including new tests, pass successfully before the Pull Request is submitted
- The team member merging the Pull Request has ultimate responsibility for ensuring that the code is properly reviewed and tested. We currently don’t have hard standards, because the scope of the changes covered in each Pull Request can vary widely. We trust our folks to make the right judgment on these issues and to use the appropriate tools to ensure that they’re making the correct call.
- Size your Pull Requests appropriately. Ideally, each Pull Request should address exactly one issue. The changes included in a Pull Request should be small enough to be reasonably handled during the review process, but large enough to let reviewers get a full picture of the proposed changes. Of course, this will vary depending on the density of your code and the relationship between your developers. One rule of thumb that we use is that the effort required to review a set of changes increases exponentially with the magnitude of the of the Pull Request. Of course, defect and entropy analysis can help you estimate the effort required for sufficient review.
- Don’t be afraid to push additional changes to the Pull Request to respond to feedback from the discussion. Commits made to the branch of a Pull Request will automatically be incorporated into the Pull Request.
- Absolutely use a new branch for each Pull Request. Further, we strongly encourage maintainers to delete the branch once a Pull Request has been successfully merged. Do not continue committing to a branch after the Pull Request has been accepted and merged. Note that this is different from the situation in the previous item. Here, we’re talking about continuing to commit changes after the Pull Request represented by the branch has been merged. This will result in one of the classic GitHub gotchas.
- Make liberal use of the ability to add comments / questions to a Pull Request – one of their primary uses for our team is fostering a conversation regarding the proposed changes.
- You have the option of forgoing Pull Requests if you’re both the owner and the only contributor to a shared repo. If nobody is going to be reviewing your changes, and if you’re following proper branch hygiene, you can probably just push directly to your own repos. At least that’s what I do… However, if I’m proposing a change to a repository maintained by another team member, I always go with a full GitHub Pull Request.
Used properly, Pull Requests are a powerful tool to help your team collaborate more effectively and function more efficiently.
Good references for Pull Requests include: