Two months ago I complained about how hard it was to get patches into PHP as the core member didn’t give much attention for pull requests on github. After posting it on the blog, I raised the issue in the developers mailing list and was suggested to join and help.
A month ago, I started sending a weekly report about new / pending / merged / closed pull requests and got cooperation from the community with reviewing the pull requests and processing them. In this period 40 requests were dealt with (obviously not all merged), and much more are work in progress while the developers are commenting on the requests.
The credit is obviously for the contributors for the patches and the core developers for the work. Stas Malyshev was of special help with this change.
You’re welcome to follow at https://github.com/php/php-src/pulls
One of the various ways to measure an open source project is the way it encourages and accepts patches from community members (or future members). The various open source projects I’m involved with have many ways to send patches: mailing list, bug tracking systems and more recently pre-commit tools for peer review (such as gerrit). Another popular option is to use github’s pull request feature for this. Personally, I find it too complex for simple patches and would prefer gerrit over it.
As part of my work at Zend, I try to make sure patches are sent back upstream. Comparing to other open source projects I know, it seems to me that PHP isn’t open enough for contributions. This might be just my private case, but if not, it would be discouraging people to contribute and blocking the community from expanding.
6 Months ago I submitted patch to #36103 (patch is at #63767), but no response. A month ago I submitted a simple patch to FPM’s shell script (#64764), and no response there as well.
To ease things up, I’ve created a github fork of the php-src repository and sent pull requests with each of the patches (as suggested at http://php.net/git-php.php). Even edited the bugs reports with the pull request details (as they provide such integration). I’m waiting to see if this will trigger a more rapid review process from the project. For the simple patch, I got a response very fast from a user, but still waiting for a core member to check it.
Going over the pull requests (both open and closed), I’m not too optimistic about having the patches merged, judging by past merged patches (most of them are at least 2 months) and by the open patches (e.g. a typos fix patch is waiting for month).
PHP 5.5 rc2 was recently uploaded to Debian unstable by Ondřej Surý. Along side the new PHP version and its features the PHP team has dealt with the JSON extension being non-free because of the “The Software shall be used for Good, not Evil” license (see debian #692613).
php5 (5.5.0~rc1+dfsg-1) experimental; urgency=low
* Add dfsg-repack.sh script to remove non-free JSON module
* Remove php5-json from Provides, since that's no longer true
-- Ondřej Surý <firstname.lastname@example.org> Fri, 17 May 2013 14:41:41 +0200
A package with a DFSG replacement for the JSON extension is waiting in the NEW queue and should be available soon. The new JSON extension was done by Remi Collet (PHP developer and Fedora’s PHP maintainer). Review and patches are welcomed, code is available at https://github.com/remicollet/pecl-json-c . I hope the PHP project will adopt the solution, after claiming they won’t fix it themselves (see php.net #63520).
One more step at being the universal operating system: getting Debian to space:
Specifically, the International Space Station astronauts will be using computers running Debian 6.
Continuing the previous post about commits and bugs, I’d like to review some mistakes I saw recently. Mistakes do happen, but mentioning them here is meant to teach others and hopefully to reduce similar ones in the future. This post isn’t about shaming the authors/commiters. Also, the points I highlight are what I consider as a mistakes or problems, other people might think differently.
- Mentioning two bugs in one commit message, which our system doesn’t support right now. So the second bug doesn’t get the commit notification, and that should be done manually (example: commit 2933935 and fdo#53278).
- Referencing gerrit changes as part of the commit message (example: commit 87f185d). Giving references as part of the commit is great and helpful, but I would prefer to see the reference to the actual commit and not to its review process. This is meaningful when you search the log. If the followup change is suggested when the first change is still in review it should be combined, otherwise it already have a commit to reference.
- Following up a commit, and not mentioning the bug it references (example: commit 87f185d). In this specific case, the we’re talking about a meta bug for translating comments from German to English (fdo#39468), so no harm done. But this important when you want to cherry pick a fix for a bug to other branches and might forget the follow up commits. It’s also relevant to more technical meta bugs (example: fdo#62096).
- Referencing a mailing list which reference a commit and a bug instead of referencing them directly (commit 21fea27, fdo#60534). This bug shows very well why correct referencing is important, a commit was made to fix the bug, a follow up commit was done without proper reference, and than the first commit was reverted. No one involved in trying to fix that specific bug knew about the follow up commit as it wasn’t documented anywhere.
- Referencing bugs though full bug URL instead of the right format (example: commit e1f6dac). Also the bug is referenced in the commit in the body of the message instead of the first line (header) which is more visible.
- Referencing non existing bugs (example: commit 3a4534b). Which got a manual notification in the bug by the comitter (fdo#33091).
- Using shortening services URL as part of commit messages (example: commit 86f8fba). There’s no way to know to what the reference is without using the service, which in this case was leading to a post on one of the projects mailing list. There isn’t any problem giving the direct URL to the list’s archive. It’s interesting whether we should link to our URL and is it “OK” to use other external services who also archive our mailing lists (example: commit e83990a).
To conclude, having references in commit message is really helpful, but please reference the right resource to help people find it easily and to let our automated services parse it.
Besides my RTL work on Libreoffice, every once in a while I just go over the regular commit log to see what have changed. I don’t necessary understand each line in the commit, but do get the general idea from the commit message. Being more dependent on the commit messages makes me review them more thoroughly (hence the topic of this post).
As many projects, Libreoffice has notifications of commits related to bugs reports when the bug number is properly mentioned in the commit message. This is very useful for other developers and also for QA people. After verification of a bug is fixed, I often use the commits listed on the bug report to cherry pick them to an older branch. Going to search for an unreferenced commit isn’t much fun.
One of the things I notice is different ways people reference bugs – from not referencing them at all to referencing them in various ways like linking to the bugs system, just writing the number (without the fdo# prefix) and other creative ways… This is also true for first time contributors, which might not know the standards or the “rules”. When I see such a case I usually put a manual notification in the bug report, and mail the author of doing so. For new or sporadic contributors this is also an opportunity to welcome and thank them for the commit and even encourage future contributions.
I have been asked why aren’t that info on the wiki, so I went looking and found out the info is in the right place under the “Development/GetInvolved” page. The relevant part is:
When you type a commit message:
- start the first line with a bug reference like fdo#12345, if you have one for your commit (see details below)
- use the rest of the first line as a concise summary of your changes
- the 2nd line remains empty
- and starting on the 3rd line you can explain how and what changes have been made for what reasons.
Thanks in advance and happy coding (:
After returning from a vacation, I went over my RTL bugs backlog. I missed helping with checking a few patches, so I got left with verifying the fixes done by the KACST guys on master. Last week was dedicated for verifications of the bug fixes, even found a regression and reverted the commit after discussion with the author.
Yesterday, I took my time with pushing these fixes also to the 4-0 branch, having them available as soon as possible instead of waiting for the 4.1 release. Total of 7 bugs got pushed, see https://wiki.documentfoundation.org/RTL_Bugs#4.0.3 for the list. I notified the developers, so they’ll know their changes are available to the public sooner than expected (and let the people enjoy their work earlier). All these pushes make sure that master doesn’t have any RTL support superiority over the current stable branch.
In one of the cases I had to do some follow up about the commit, finding another relevant commit. This was merged with a trick Eike taught me a while ago for squashing commits with git rebase. Fridrich quickly helped with verifying that the squash stills looks good, letting me push another dependent fix on top of it.