Managing Pull Requests for the MongoDB PHP driver
The MongoDB PHP Driver is hosted as an Open Source project through GitHub. Everybody can check-out the repository and provide patches to this. In order to make things easier to manage, even Hannes, Jeremy and me, the maintainers of the extension will only merge code into it through patches.
To streamline the managing of those patches we are using GitHub's Pull Request (PR) system. Any time we fix a bug, or add a feature, we create a branch of our personal fork and when we are done, we submit a pull request to the master repository at http://github.com/mongodb/mongo-php-driver. Before this pull request gets merged, a few things happen. First of all, we make sure that there is a related issue in JIRA. If there is no issue, then one has to be created. Also we verify the patch against our contributing guidelines - including whether the patch can actually automatically be applied to the branch.
Once the pull request is deemed okay, we review the code and the test cases that are required to be submitted as part of the pull request. In our case, I usually review Hannes' patches, and Hannes reviews mine. GitHub's PR system allows us to easily add comments to bits of the patch which is mighty useful. Of course, as part of the review process we need to be able to compile the code and run the test cases as well. For this purpose I have written a small bash function to help me out:
function mongoprfetch()
{
git checkout master
git fetch origin pull/$1/head:pr/$1
git checkout pr/$1
git rebase master
}
This function is run on the command line (in the checkout of mongodb/mongo-php-driver) with as arguments its Pull Request number:
mongoprfetch 206
The script will checkout master, fetch the PR as it was a branch from GitHub and rebases it against master. This allows us to test whether the PR cleanly applies to the latest code in master. Without the rebasing, the commit history would also look really complex as this figure shows:
When all review comments are resolved, the code compiles and the test cases succeed, it is time to merge the patch. For this, I am using another small bash function to help me out:
function mongoprmerge()
{
git checkout master
git merge --no-ff -m "Merged pull request #$1" pr/$1
git branch -D pr/$1
git push
}
Because we already have rebased against the latest master branch when running mongoprfetch 206, a merge would mean that we lose the ability to see which commits belonged to the PR - as Git would simply use "fast forward":
We find this undesirable and hence we specify the --no-ff option to git merge as you can see in the mongoprmerge function. This produces histories that look like:
The only thing that we have not automated as part of merging pull requests is closing the associated Jira tickets as well.
Right now we have no specific scripts to handle also the stable branch (v1.2) as we have been working hard to reimplement the connection handling stuff in the PHP driver. I will write more about the changes in the driver, as well as managing a merging to/from a stable branch in future blog posts.
Comments
This is a really nice workflow. I am using something like this in my ZSH shell (oh-my-zsh plugin): https://github.com/paulredmond/oh-my-zsh/commit/d0cded4a112faf32fd448906b5709725d54f63c7
Still waiting for a PR merge, but very handy. I find that I get lazy and this is a nice simple workflow for running a PR through its paces before merging.
Life Line
Merged pull request #1055
Fixed issue #2387: Remove INI entries for changed and removed Xdebug …
Merged pull request #1053
Reimplement PR #1052 with normal style
Add missing section comment
Merge branch 'xdebug_3_5'
Merged pull request #1054
Change error retrieval method in ctrl_socket.c
Pink Sky at Sunset
I took this photo over the Christmas period in the Dutch city of Breda.
I walked 8.5km in 1h25m28s
I walked 8.1km in 1h21m10s
I walked 0.8km in 9m03s
I walked 4.8km in 50m12s
Went for a 20k walk through Bushy Park, along the Thames, and through Richmond Park and Wimbledon Common. It was a bit nippy!
I hiked 19.3km in 3h52m02s
Updated a pub
I walked 4.6km in 44m50s
I walked 4.9km in 47m58s
Update Westbourne Green area, now that it is open
I walked 11.9km in 2h3m03s
I walked 9.8km in 1h47m38s
I walked 10.2km in 1h34m25s
Whoop! FOSDEM travel and hotel booked. See you in Brussels at the end of January?
I walked 10.6km in 1h48m23s
I walked 3.0km in 33m38s



Shortlink
This article has a short URL available: https://drck.me/managepr-9ll