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 #1029
Reflow some comments
Add comments, add end of file newlines, fix php 8.5 compilation
Benchmark Xdebug performance
Merged pull request #1051
PHP 8.6: printf() is now optimised out if it only uses %s and %d (and…
PHP 8.6: The object IDs of objects changed in tests
PHP 8.6: ZSTR_INIT_LITERAL() no longer takes non-literals, so use zen…
PHP 8.6: WRONG_PARAM_COUNT has been removed in favour of zend_wrong_p…
PHP 8.6: zval_dtor() has been deprecated in favour of zval_ptr_dtor_n…
Update test for version constraints, as well as the error messages
I walked 7.4km in 1h38m15s
I just watched an interesting and lovely documentary on BBC Four: The Magical World of Moss.
It's on the iPlayer for another month:
https://www.bbc.co.uk/programmes/m001hqthI walked 8.2km in 1h26m34s
Merge branch 'xdebug_3_5'
Document to use PIE instead of PECL in the README
I walked 9.2km in 1h45m44s
I walked 10.1km in 1h40m23s
If we must show off a little…
In more info innocent times, the hat riots caused some spankings...
100 Years Ago Men and Boys Fought on the Streets of New York Over Wearing Straw Hats Past Summer | The New York Public Library
https://www.nypl.org/blog/2022/09/23/straw-hat-riots-nyc


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