The PHP project and Code Review
Reading code is not only fun, its also a great way to exercise your brain - not to mention a fantastic way to discover new ways to solve problems. At work (we are hiring btw!), for example, I read pretty much every single commit (and merge requests, for that matter) - and I'm subscribed to several different OSS commit lists. I can't say I read every commit to PHP, I focus on the areas I care about, but I do skim over the rest - if only just to see when new features are added.
The PHP project has a good chunk of mailinglists, everything from support lists, developer discussions, QA, and so on. Every commit to our SVN is automatically posted to the relevant commit mailinglist(s).
The main commit lists are;
- PHP (php-cvs@lists.php.net, http://news.php.net/php.cvs)
- PHP Documentation (doc-cvs@lists.php.net, http://news.php.net/php.doc.cvs)
- PHP-GTK (phpgtk-cvs@lists.php.net, http://news.php.net/php.gtk.cvs)
- PEAR (pear-cvs@lists.php.net, http://news.php.net/php.pear.cvs)
- PECL (pecl-cvs@lists.php.net, http://news.php.net/php.pecl.cvs)
Then we have specific lists for each translation of the docs, all the websites, PEAR docs and so on.
The great thing about these commit lists is that *anyone* can subscribe to them, and quite a lot of people actually are.
For example, there are over 200 people registered to the PHP commit list, and over 100 people on the documentation commit list alone. That is ca 25% of the people that are subscribed to their discussion counterparts.
Granted that most of the subscribers do not actively review the commits, I guesstimate there is still a sizable percentage of them that do.
Just the simple fact that I know people will be reading through my commits makes me think about what I am doing a bit more; "Is this really needed?", "Is there be a better way solving this?", "Could it potentially break other things?", "Is this actually correct?"..
The people who review the commits often don't seem like the friendliest people in the world.. If there are issues with the commit; You will be told. No doubt about it. And you will feel like an idiot, for few minutes, for not having caught it yourself - and promise yourself to review your commits better in the future. Learning from your mistakes. I love it. Mission accomplished.
Last year I updated the credit list, printed out by phpinfo() & phpcredits(), for the people maintaining our websites, with the commit message "Throw some credit around" (yes, the typo and incorrect colspan in the commit was caught and fixed).
Fast forward 15months, to last Friday. Another commit from me to PHP trunk with the commit message "Throw some credit around", and then to PHP 5.3 a minute later with the same commit message.
Less then 10minutes after the commit I had received 2 emails asking me "WTF?" - and a poke on IRC.
I had no idea what those guys were talking about. None what soever. I had not committed anything to any PHP project for several days. Nothing. Not even a typo fix.
Turns out that someone in Asia had somehow managed to get his/hers hands on my PHP.net account credentials.
Interestingly enough the commit doesn't introduce any security holes, bugs, or anything at all really. It just adds "Wolegequ Gelivable" to the credit list (whatever that means).
Its not a great feeling to have your account hacked into, but I do wonder what the intentions were.. Maybe just an credentials check, which was supposed to be followed by evil commits if noone had spotted the first one? The Chinese government trying to introduce security holes so they can break into PHP websites?
In any case. It took less then 10minutes for 3 people to catch it, that is pretty cool.
-Hannes
p.s. Last year, one of my new year's resolutions was to "blog once a month". My last blogpost was in January.
The PHP project has a good chunk of mailinglists, everything from support lists, developer discussions, QA, and so on. Every commit to our SVN is automatically posted to the relevant commit mailinglist(s).
The main commit lists are;
- PHP (php-cvs@lists.php.net, http://news.php.net/php.cvs)
- PHP Documentation (doc-cvs@lists.php.net, http://news.php.net/php.doc.cvs)
- PHP-GTK (phpgtk-cvs@lists.php.net, http://news.php.net/php.gtk.cvs)
- PEAR (pear-cvs@lists.php.net, http://news.php.net/php.pear.cvs)
- PECL (pecl-cvs@lists.php.net, http://news.php.net/php.pecl.cvs)
Then we have specific lists for each translation of the docs, all the websites, PEAR docs and so on.
The great thing about these commit lists is that *anyone* can subscribe to them, and quite a lot of people actually are.
For example, there are over 200 people registered to the PHP commit list, and over 100 people on the documentation commit list alone. That is ca 25% of the people that are subscribed to their discussion counterparts.
Granted that most of the subscribers do not actively review the commits, I guesstimate there is still a sizable percentage of them that do.
Just the simple fact that I know people will be reading through my commits makes me think about what I am doing a bit more; "Is this really needed?", "Is there be a better way solving this?", "Could it potentially break other things?", "Is this actually correct?"..
The people who review the commits often don't seem like the friendliest people in the world.. If there are issues with the commit; You will be told. No doubt about it. And you will feel like an idiot, for few minutes, for not having caught it yourself - and promise yourself to review your commits better in the future. Learning from your mistakes. I love it. Mission accomplished.
Last year I updated the credit list, printed out by phpinfo() & phpcredits(), for the people maintaining our websites, with the commit message "Throw some credit around" (yes, the typo and incorrect colspan in the commit was caught and fixed).
Fast forward 15months, to last Friday. Another commit from me to PHP trunk with the commit message "Throw some credit around", and then to PHP 5.3 a minute later with the same commit message.
Less then 10minutes after the commit I had received 2 emails asking me "WTF?" - and a poke on IRC.
I had no idea what those guys were talking about. None what soever. I had not committed anything to any PHP project for several days. Nothing. Not even a typo fix.
Turns out that someone in Asia had somehow managed to get his/hers hands on my PHP.net account credentials.
Interestingly enough the commit doesn't introduce any security holes, bugs, or anything at all really. It just adds "Wolegequ Gelivable" to the credit list (whatever that means).
Its not a great feeling to have your account hacked into, but I do wonder what the intentions were.. Maybe just an credentials check, which was supposed to be followed by evil commits if noone had spotted the first one? The Chinese government trying to introduce security holes so they can break into PHP websites?
In any case. It took less then 10minutes for 3 people to catch it, that is pretty cool.
-Hannes
p.s. Last year, one of my new year's resolutions was to "blog once a month". My last blogpost was in January.
Hey Hannes, i'm one of the student who participated in GSoC 2009. Could you see if PHP may use the CodeReview project at http://code.google.com/p/rietveld/ ??? (It is also used by the chromium project)
ReplyDeleteWe don't have an formal CR process, and I doubt that will change anytime soon..
ReplyDeleteapparently this bad guy is from China
ReplyDeleteWolegequ and Gelivable are very popular internet slang in China since 2010.
Wolegequ[我了个去] means F word
Gelivable [给力] means very good or wonderful
Its really becoming a menace. We need to have proper security checks for such things else things may turn out to be quite tough in coming times.
ReplyDeleteWolegequ Gelivable == This is f**king awesome!
ReplyDeleteI guess the hackers just want to warn the vulnerability of php.net, since they were trying to contact the php team to report the issue, but anyhow they failed due to some unknown reason.