Pull requests form one half of the fork-pull workflow, but what is a pull request?
When you fork a repository, you normally carry out development work on the fork. When that work is finished, you request that it is pulled back into the repository. This request is known as a pull request.
The benefits of using pull requests are that once opened, you can automate testing, peer review the changes, and decide how to best merge it, or not!
We Use Pull Request For Everything, Almost
Here at RhodeCode we use pull requests for all contributions to a repository. Except of course the documentation repository, which is like a goat rodeo half the time, leaving me the joyous task of tracking down some opinion pieces masquerading as fact! If you want, you can check out our workflow breakdown from a previous blog.
Once a pull request is opened, a reviewer is assigned to it from the team and starts checking it to ensure code quality stays high. This usually leads to a combination of inline comments, and a discussion about what is going into the change.
Inline Comments
More often than not, a lot of drawing on whiteboards with different coloured markers, and adding two lines of code is required to fix the issues raised in a comment.
Inline comments have proven to be a great way to leave feedback and increase collaboration around changes, as it keeps feedback tight and on topic, most of the time!
Updating Pull Requests
Once you have made your changes, and dealt with the various pedantic and oft meandering thoughts that find their way into a code review, you can push them up to your fork and update the pull request from the RhodeCode Enterprise interface.
After updating, the review can continue based on the latest changes. One important thing to note is that the review comments are all returned to the under review status after an update. This helps ensure no one sneaks some code past the gatekeepers!
Server Side Merging
Once you eventually managed to please all the reviewers and get approval, you can then automatically merge it from the interface. That little blue button is so handy!
Review Checklists
As with all tools though, using them wisely is the most important part of the puzzle. We have a number of checklists depending on the project and these form the minimum basis for a peer review. Here are the most basic requirements to have a pull request accepted:
- The pull request should have a reasonably clean history.
- Commits messages like Fix typo in last commit should be squashed.
- All tests must be passing.
- It must be peer review approved. Those green lights have got to be flashing!
- Preferably only have one commit per topic. If a commit has a message like Fixes A and B and C and everything else, then it's a good indicator that too many things are in one commit.
As an aside, this final point is why I have had to learn hg histedit. Some people don't appreciate my poetic approach to commit messages!
But there is some logic to squashing commits which will be covered in a future post.
Obviously, if making front end changes, or security changes, there are different checklists. Here is an excerpt from the front end changes checklist. It is both surprising and not surprising to see Internet Explorer mentioned in there. IE testing has really become the modern day latin grammar lesson for developers.
- Check changes in IE and standard browsers.
- Check around the changes and ensure that there are no style regressions.
- Ensure any changes are covered by the style guide.
Merge Options
Once all that has been worked through, you merge the pull request into the target repository. RhodeCode Enterprise gives you the option to set what kind of merge strategy you would like to use.
Depending on the preference of the repository owner, you can either use the normal merge procedure, or rebase on top of the current commit.
Conclusion
The fork-pull workflow is a great way to double check code quality, and test changes before you merge them with your feature work. When used correctly, this keeps bugs low, and promotes collaboration among team members.
Personally I find that helps both my productivity and learning. There is nothing like a smart colleague looking over your shoulder to motivate you to not make mistakes!
Getting the collaboration mix right comes down to you though. You need use the right tools, and have some fun using them.
We are pretty happy with this aspect of RhodeCode Enterprise, but not ones for resting on our laurels, we will be continually improving the functionality. What's nice, is that we see our progress in action as we deploy the latest changes to RhodeCode Enterprise multiple times throughout the day. This allows us to enjoy the changes immediately, or complain strongly about them over
Slack!