Tuesday, December 30, 2008

Summary of December Sponsored Twisted Development

With a lot of help from Allen Short, two more weeks of sponsored Twisted development have just been completed. During this round, I continued to work on the new HTTP client and spent some time trying to resolve some long-standing issues around Twisted's support for starting and controlling child processes. Meanwhile, Allen developed a plan for integrating the SIP code, maintained by Divmod, Inc. outside of Twisted for several years, into Twisted and the existing SIP support in Twisted.

The SIP-related work that Allen did encompassed these tickets:

#2194 - small bug in SIP Via header generation
#3575 - Common implementation of RFC 2617 digest authentication
#3582 - Improve SIP URI parsing/formatting
#3583 - Include SIP message-parsing changes from Sine
#3584 - SIP transport layer and transaction layer

These are the process tickets that I worked on:

#733 - twisted's SIGCHLD handler breaks popen
#1997 - perhaps wakeUp could be slightly simpler (closed)
#2967 - Reaping child processes has superlinear complexity on POSIX
#3571 - intermittent spawnProcess failure in test_process on Linux (closed)
#3576 - add high-level, cross-platform close-on-exit togglers to t.i.fdesc

A lot of this work consisted of getting some old code which Glyph had previously worked on but never completed updated and brought closer to being ready for a real code review. The summary of #1997 is a bit misleading - while the change did simplify the reactor's "wake up" mechanism, it also removed a race condition, fixing a bug which could cause certain events to fall into a limbo where they could only ever be processed after another event arrived.

Aside from process related tickets and the HTTP client, I worked on a few other tickets as well:

#2808 - AMP should raise MissingArgument (or other) if a callRemote is called with wrong arguments (closed)
#3246 - remove all mentions of plugins.tml from the documentation (closed)
#3562 - Setup Python 3.0 buildslave(s) (closed)
#3568 - ERROR from conch test when pycrypto is not installed
#3569 - Twisted Web WSGI container sometimes emits too many (or duplicate) headers (closed)

The Python 3.0 buildslave will probably garner the most interest of the bunch. The resolution of this ticket does not mean that Twisted supports Python 3.0 now. It just means that we've added a column to our buildbot (continuous integration system). We can now tell at any given time that Twisted does not support Python 3.0. ;) But seriously, with this slave set up, we can accept contributions which move Twisted towards Python 3.0 compatibility, but I don't plan to spend any time doing such development myself in the near future. There's a ton of other more pressing issues, so I'll leave Python 3.0 work to people who think they'll benefit from it.

As for the new HTTP client, this round of development moves it inexorably towards completion. Itamar Shtull-Trauring and I spent several days improving its error handling, simplifying some of the more hideous parts of the implementation, and dealing with various corner cases (and HTTP 1.1 sure has a lot of them). The development branch also includes a sketch (only a sketch) of the higher-level API we're planning to provide on top of the low-level protocol implementation (the sketch is currently undocumented and a bit obtuse, so it may not make sense without me or Itamar looking over your shoulder) and an example which uses the APIs provided by the low-level protocol to implement a simple web client (something along the lines of wget or curl, but obviously much, much, much more rudamentary).

That's it for now. It'll be 2009 when I post the next one of these. 2008 has been a great year for Twisted development, and I know that things are just going to get better. :) Thanks again to the Software Freedom Conservancy, all of the Twisted Sponsors, and all the other developers who contribute to Twisted.

Wednesday, December 3, 2008

Summary of November Sponsored Twisted Development

I've just completed another round of sponsored Twisted development (the sixth so far). This period sees a lot of documentation improvements, as well as various bug fixes and continued work on a new HTTP client for Twisted Web (#886).

These are the documentation issues which were resolved:

#2281 - Annotations for Twisted Finger Tutorial
#3548 - twisted.conch.client.knownhosts.PlainEntry misdocuments its "_hostnames" attribute.
#3455 - CONNECTION_LOST not an Integer...
#3537 - Conch's public key authentication process is confusing.
#3490 - FTPClient errors should provide ftp errorcode

There were some improvements to Twisted Web:

#1878 - twisted.web.monitor traceback_ AttributeError: class IChangeNotified has no attribute '__class__'
#2402 - client.py crashes on URL's that would be no problem for most browsers
#3192 - HTTPClientFactory sets followRedirect on the HTTPPageGetter class
#3469 - Exception is rendered when NotFound is more appropriate.

A problem with Conch's SFTP server's reporting of modification times was fixed (and then a problem with the unit tests for the fix was fixed):

#3503 - Wrong date format delivered by twisted.conch.ls.lsLine
#3551 - TZ=Pacific/Auckland python2.4 ./bin/trial twisted.conch.test.test_cftp.ListingTests fails

With #3551, I was once again reminded that working with timezones is extremely challenging. This time, I found that the range of local renderings of any particular UTC timestamp is greater than 24 hours, so you cannot rely on any particular timestamp falling on a particular day in an arbitrary timezone. So like other unit tests in Twisted, the unit tests for lsLine explicitly set the timezone when they want to make assertions which depend on it, then reset it to its original value. Unfortunately, this means they can't run on Windows, since Windows apparently lacks APIs for doing this. I also rediscovered that time.tzset() is a no-op, at least on Linux (but the unit tests call it anyway, in case Solaris or HP-UX or some other POSIX implementation requires it).

An important issue with Twisted's Jabber support was fixed as well (#3463). This one prevented Twisted's Jabber client from successfully negotiating TLS when connected to Google Talk (and possibly other Java-based Jabber servers). This problem was ultimately caused by a bug in the TLS support on Google Talk which caused TLS negotiation to fail if the client included a session ticket (RFC 5077) section in the handshake. This is allowed and servers which do not support session tickets should ignore the section, but for some reason it causes problems with Google Talk. OpenSSL (on which Twisted's SSL support is based) 0.9.9 enables session tickets and the 0.9.8 package distributed with some platforms (eg Ubuntu 8.10) includes a backport of this feature. So Twisted's Jabber client cannot communicate with Google Talk if one of those versions of OpenSSL is installed.

A couple issues related to Python 2.6 support were fixed:

#2763 - md5 and sha module will be deprecated in python 2.6
#3545 - Support Python 2.6 in the Windows build system.

A bug in Twisted Mail's IMAP4 client which prevented the unseen part of a server's response to a select or examine command from being made available to applications was fixed (#3550).

And there were several other assorted fixes:

#3521 - Documentation for `processExited()` conflicts with the implementation
#3315 - t.p.reflect.safe_repr includes the wrong traceback and misformats the return value
#3541 - twisted.internet.abstract.FileDescriptor.loseConnection drops reason (will be reported as clean shutdown)
#3544 - bin/admin/change-versions should update the main README file

I also spent some time working on converting Twisted Lore from using microdom, Twisted's XML parser and DOM implementation (circa 2002), to using minidom, the XML parser and DOM implementation in the Python standard library. For a long time, microdom was better than any of the alternatives, but it's seen very little maintenance in the past several years and there are some problems with Lore (eg #414) which are caused by behavior of microdom that it would be difficult to change.

Unfortunately, switching to minidom brings a new set of problems. It's still probably worthwhile, but it seems like it's harder to use than it should be. Some of the issues I've run into so far (and I'm not done yet):

  • minidom's constructors are less convenient than microdom's constructors. For example, the Text constructor doesn't accept a string to use as the text node's value. Instead, you have to instantiate Text and then set an attribute. This expands code which should have been one line into three lines. And to make things worse, a Text instance with no data set raises an exception from the __repr__ method.
  • The parse error exceptions raised by expat are less informative than the parse error exceptions raised by microdom. For example, if a document contains mismatched tags, microdom reports the name and location of both the opening and closing tags. minidom will report only the location of the closing tag. It's easy enough to find out the name of the closing tag by finding that location in the input document, but finding the offending start tag means parsing the document in your head. For any non-trivial document this is ridiculous. Fortunately, by switching to sax and providing custom error and content handlers, most of the information can be recovered. This information is always useful though, and it would be better if minidom provided it by default.
  • Once you switch to xml.sax, you have to remember to disable its validation features or it will try to retrieve DTDs from the internet every time you parse something. This is bad, bad default behavior.
Some of these issues may turn into Python bug reports once I've made more progress on converting Lore. A much bigger difficulty with the conversion than the problems minidom has is the fact that Lore is largely pre-UQDS code. Some of it has tests, but they're mostly whole-system tests which compare gigantic xhtml strings and are subject to extremely obscure failures. And most of it doesn't have any tests at all.

Switching away from microdom should be worth the effort though. minidom is a bit faster and Lore will generate better looking output once the switch is done.

Itamar points out I didn't separate tickets into groups for those I reviewed and those I did development on myself this time. So let me point out that of the above tickets, many were developed by others and reviewed by me. The Twisted development process is highly collaborative. I couldn't accomplish anything without the help of all the other great Twisted developers who volunteer to contribute to Twisted in their free time. If you want to find out which are which, head over to the Twisted issue tracker where you can look up the development(/authorship/etc) history of any ticket.

That's all for now. Thanks again to the Software Freedom Conservancy, all of the Twisted Sponsors, and all the other developers who contribute to Twisted (hi Michael!).

Monday, November 3, 2008

Summary of October Sponsored Twisted Development

Hello again,

I've just spent another two weeks on sponsored Twisted development. This round sees 25 tickets resolved. Considering the fact that two tickets in particular took up a lot of development time, I'm pretty happy with this progress. I did a lot of reviews for other people this time around. Here are some of the more interesting things developed by other people to which I gave the thumbs up:

#2026 - Edit the twisted.internet docstring to make contents clearer
#2902 - bad flag value in twisted.conch.ssh.filetransfer
#3335 - FTPClient should support renaming files
#3491 - FTPClient should support deleting files
#3500 - Add directory creation method to FTPClient

(The other tickets I reviewed which weren't quite as as interesting were: #532 #745 #1853 #2281 #2375 #2682 #2748 #3197 #3216 #3315 #3381 #3393 #3439 #3464 #3466 #3471 #3482 #3486 #3504 #3505 #3507)

And here are some of the more interesting ones I worked on myself (bold tickets are now closed):

#886 - Rewrite twisted.web.client.getPage to support streaming of result data and returning headers etc
#2346 - Header generation not conforming to RFC822 and RFC2046
#2605 - stdlib unittest change breaks a trial test
#3298 - twisted.internet.defer.FirstError masks errors when logged
#3329 - HTTP's #(...) syntax allows null contents
#3477 - BinaryBoxProtocol.makeConnection calls startReceivingBoxes too early
#3478 - AMP should enforce MAX_KEY_LENGTH in the protocol parser
#3487 - Add TestCase.flushWarnings

(The other slightly less exciting tickets I worked were: #3169 #3197 #3216 #3515 #3519)

#3487 and #886 took up a lot of my time this round. With #3487 resolved, the way is clear for the next Twisted release to run well on Python 2.6 (ie, there are no more known issues). Once I got that out of the way, I moved back to web issues. #886 is going to represent a big step towards having a good HTTP client API in Twisted. Anyone who has tried to use twisted.web.client for anything non-trivial knows how much work is needed in this area. :)

As usual, this work is made possible by the SFC and all of the sponsors who made this possible, as well as to all the other Twisted developers who helped out by writing or reviewing code!

That's all until next time.

Friday, September 26, 2008

Summary of September Sponsored Twisted Development

Another round (the fourth) of sponsored Twisted development has just wrapped up.

I had a lot of excellent help again from Thomas Hervé and Thijs Triemstra. Thomas is a great reviewer and Thijs has been making steady progress on resolving lots of documentation tickets.

Here are the tickets I spent time developing:

#1068 - UNIX Port creation has race condition in permission setting code
#1382 - twisted.web.client.HTTPClientFactory sends Host header (and others) more than once
#1903 - Twisted doesn't reset timeout when getting POST data
#2542 - twisted.web.microdom.Node.isEqualToNode does no actual comparison
#2683 - twisted/web/http.py has self._header instead of self.__header
#2878 - Intermittent unclean errors from twisted.test.test_ftp.FTPServerPasvDataConnectionTestCase.testTwoDirLIST on OS X
#2947 - intermittent twisted.test.test_pb.NSPTestCase.test_NSP failure on OS X
#2974 - twisted.vfs.test.test_vfs.OSVFSTest.setUp does not close the files it opens
#2976 - twisted.mail.smtp.xtext_encode and xtext_decode take the wrong number of arguments
#3133 - Whois function for the ircclient class
#3211 - twisted.python.log.showwarning doesn't take the new line argument added in Python 2.6
#3222 - Intermittent failure of twisted.test.test_twistd.AppProfilingTestCase.test_profileSaveStats on Windows
#3223 - twisted.trial.unittest.TestCase.assertWarns always fails when the C warnings module_ new in Python trunk_ is in use
#3239 - Intermittent failure of twisted.test.test_iutils.UtilsTestCase.testOutputWithErrorIgnored on Windows
#3266 - Provide tools for managing new deprecation policy
#3402 - Intermittent failure of twisted.test.test_tcp.LocalRemoteAddressTestCase.test_hostAddress on OS X
#3404 - twisted.test.test_process.ProcessTestCase.testManyProcesses fails on WinXP
#3416 - rename `twisted.web2.iweb.IOldRequest` to `twisted.web.iweb.IRequest`
#3424 - Intermittent failure of twisted.names.test.test_names.ServerDNSTestCase.testZoneTransfer on OS X
#3426 - twisted/internet/test/test_process.py tests which might fail while waiting for an event should use runReactor instead of reactor.run

As you can see, work improving Twisted Web continues here. There's also been some long-needed fixes to tests in the suite which fail intermittently on various platforms. With more of these problems resolved, it gets easier and easier to determine if new proposed changes introduce any regressions.

Of course, I reviewed a lot of tickets over the last few weeks as well. Here they are:

#532 - Big jump from finger18.py to finger19.py in tutorial.
#1124 - getHost and getPeer not needed for IProcessTransport
#1157 - web.http.HTTPClient does not support chunked transfer-encoding
#1262 - Document Twisted copyright policy
#1328 - People keep asking about _trial_temp
#1852 - Replace uses of @ivar in interfaces with z.i.Attribute
#1853 - Fix/eliminate @cvar in Interfaces.
#2026 - Edit the twisted.internet docstring to make contents clearer
#2500 - Add @since to the coding standard
#2514 - Documentation bug in the list of possible classes
#2555 - Documentation update: deferred results in PB
#2909 - Document XXX Comment Policy
#3045 - Coding standard should forbid the use of "foo %s" % notATuple
#3197 - IProcessTransport.pid is incompletely documented
#3236 - Remove the FAQ from doc/core/howto_ since it is a duplicate of the FAQ wiki page
#3254 - twisted.python.deprecate does the wrong thing for methods
#3315 - t.p.reflect.safe_repr includes the wrong traceback and misformats the return value
#3382 - coding-standard.xhtml mislinks to twisted.python.compat
#3414 - Coding standard refers to old tools
#3415 - Get rid of X{} references in docstrings
#3422 - Remove acceptance tests in coding standard
#3423 - Get rid of references to old admin tools
#3435 - XmlStream should be convenient to use in a server context where there should be one authenticator per connection
#3446 - Infinite loop/memory-usage in irc.split()

These tickets represent a lot of good work by other Twisted developers (and you can see there are a lot of documentation tickets here, almost all the work of Thijs!).

I also spent a bit of time cleaning up our issue tracker. When you've been keeping track of bugs and feature requests for more than five years, it's hard to avoid having a certain amount of invalid tickets pile up. It's a bit of a drag going through the issue tracker to try to find these, but it's well worth while (and I can't expect everything to be fun ;).

That's it for this time. I'll be back with another report in about a month.

Tuesday, August 5, 2008

Summary of July/August TSF Sponsored Twisted Development

I've just completed the third two-week period of TSF sponsored Twisted development.

Tickets I worked on during this period:

#686 - [TEST] startTLS is broken if there's already data in the outgoing buffer.
#966 - [PATCH] Add --umask option to twistd
#1200 - twisted.test.test_internet calls reactor.iterate()
#1493 - static File web module doesn't support byte ranges
#2276 - Changes to TwistedNames to make it support NAPTR records
#2338 - trial should handle concurrent usage in the same directory gracefully
#2753 - twisted.web WSGI support
#2790 - UDP Transport write() raises socket.error EWOULDBLOCK
#2845 - twisted.internet.thread._putResultInDeferred should be public
#2931 - FilePath.setContent writes and renames but does not sync
#3342 - twisted.names dns-spoofing vulnerability
#3347 - twisted.names dns-spoofing vulnerability (birthday paradox)
#3367 - twisted.python.lockfile.FilesystemLock.lock fails with EEXIST

Tickets I reviewed during this period:

#637 - Odd filenaming in tutorial/intro.xhtml
#638 - Allow overriding twistd's logging options
#689 - twistd man page needs a section on signals
#1246 - reactor.callWhenRunning is not in the Using Processes document
#1253 - Create index.xhtml files for non-core doc trees
#1490 - Allow twistd to "run" packages_ e.g. 'twistd run mypackage --port 8080'
#1821 - Turn deferredgenerator wiki page into howto
#1888 - Review man pages
#1971 - Links in SEE ALSO section of doc/lore/man/lore.1 are bogus
#2208 - Standardize on the Python shebang line
#2375 - these objects' docstrings are not proper epytext:
#2438 - Get rid of references to maintainer email addresses from code
#2607 - conch.checks.SSHPublicKeyDatabase calls os.seteuid/os.setegid even if it's not necessary
#3244 - runInteraction exceptions are swallowed if rollback fails
#3254 - twisted.python.deprecate does the wrong thing for methods
#3285 - Add ISUPPORT implementation for irc.py
#3332 - SSHAgent implementation_ unit tests
#3355 - t.application.app.AppProfiler handles options oddly
#3365 - a few more IRCClient docstrings
#3366 - add IRCClient.back()
#3377 - words.protocol - irc.py assumes nickchange to be successful before server ack

Allen Short also helped out with a few reviews this time. Thijs Triemstra, a new Twisted contributor, has also been very active lately working though documentation tickets and making great headway.

Twisted's open ticket count also fell below 1100 this week. For the last three months, for the first time ever, we've been able to consistently resolve more tickets than have been filed.

Thanks to the SFC (<http://conservancy.softwarefreedom.org/>) and all of the sponsors (<http://twistedmatrix.com/trac/wiki/TSF/FoundingSponsors>) who made this possible, as well as to all the other Twisted developers who helped out by writing or reviewing code.

Thursday, July 10, 2008

June/July TSF Sponsored Development

The second (of what should end up being many) round of TSF sponsored development has just wrapped up. Like last time, this was two weeks of work by yours truly (with an afternoon of Glyph's time spent reviewing code to keep things moving). This time around there were fewer tickets waiting to be reviewed when I got started, however there was still plenty of code to look at. Over the full period, I reviewed:

#1144 Documentation: twisted.internet.reactor does not appear in the API docs
#1253 Create index.xhtml files for non-core doc trees
#1255 Update copyrights in the man pages
#1878 twisted.web.monitor traceback_ AttributeError: class IChangeNotified has no attribute '__class__'
#1890 move examples from core into correct sub-packages
#1900 Error in documentation online
#2169 twisted.plugin documentation errors
#2208 Standardize on the Python shebang line
#2438 Get rid of references to maintainer email addresses from code
#2552 broken links in intro.xhtml
#2607 conch.checks.SSHPublicKeyDatabase calls os.seteuid/os.setegid even if it's not necessary
#2716 Eliminate relative imports from twisted.conch
#2815 Update VFS backends to an async interface
#2821 create twistd plugin for vfs
#2845 twisted.internet.thread._putResultInDeferred should be public
#3182 Trivial typo in twisted.internet.protocol
#3257 Rewrite twisted.web.static.File.directoryListing to not use woven
#3269 curses.setupterm must only be called once per process
#3300 twistd should support setting the syslog facility
#3326 Typo in gtkmanhole.py

Other tickets I worked on included:

#1069 log observers that throw exceptions should not be removed
#1152 xmlrpc.html doesn't describe how to return errors to the client
#1291 Expose "process exited" hook on ProcessProtocol_ turning current processEnded into user-overriddable behavior
#1493 static File web module doesn't support byte ranges
#2303 Deprecate setUpClass and tearDownClass and_ if possible_ fix the subclassing behaviour.
#2327 Intermittent failure in PB tests
#2631 Update coding standard to indicate preference for TestCase methods which being with "assert" and which do not have an underscore in their name
#2874 _sslverify.problemsFromTransport should be deprecated
#3029 documentation for twisted.python.deprecate.deprecated is incomplete (and other sundries)
#3059 twisted.internet.tcp.Client.getPeer incorrectly returns hostnames
#3116 Errors in processEnded can cause processes to be eternally reaped
#3159 t.i.utils process functions should have a default cwd of None_ not '.'
#3218 SSL disconnection sometimes hangs indefinitely with pyOpenSSL 0.7
#3255 Trial fails to display the line where the error occured in case of SyntaxError
#3300 twistd should support setting the syslog facility
#3301 superfluous local in AMP.__init__
#3305 CR IAC ignored in TelnetClient
#3306 twisted.test.test_ssl.StolenTCPTestCase has a number of defects
#3339 mailmail raises an exception instead of giving an error message

In particular, I spent a lot of time on #1291 which will make it possible to control child processes more precisely with Twisted by separating out the notification that a child process has actually exited from the notification that all of its file descriptors have been closed.


Thanks to the SFC (<http://conservancy.softwarefreedom.org/>) and all of the sponsors (<http://twistedmatrix.com/trac/wiki/TSF/FoundingSponsors>) who made this possible, as well as to all the other Twisted developers who helped out by writing or reviewing code.

Wednesday, July 2, 2008

Twisted in the News

Everyone's Hiring Twisted Talent

The jobs category made it to the top of the list this time around, with folks like Rackspace hiring Knowers in the Ways of Twist. This automation engineer position puts Twisted on par with .Net and C#, giving it title space. This one is looking for talent working with custom protocols and mobile device products. Fluidinfo is also looking for some Twisted talent; if you're interested, email me and I'll put you in touch with them.

The Rackspace job posting is interesting, because a reliable source said that they are doing some very sexy stuff with Twisted and cloud computing. This would be quite the catch, job hunters...

The Fredericksburg Twist

Zope Corp recently released zc.twist 1.3, "Talking to the ZODB in Twisted Reactor Calls." Their page on PyPI has extensive documentation and some great examples. With their Zope 3 work, Zope Corp has consistently provided some of the best technical documentation I've seen in the Python world (possibly fueled by their excellent use of doctests). This release makes me want to jump back into the ZODB :-)

Related to the ZODB, be sure to read Martijn Faassen's recent post.

Compliments

Terry Jones referred to the Twisted crew nicely in his blog post, saying that the framework is "an extraordinarily good set of asynchronous networking libraries written by a set of extraordinarily young and gifted programmers." Well, we're starting to get grey hairs, bald spots, and love handles... so perhaps we're not quite as young as we used to be ;-) But we'll take the compliment anyway, Terry ;-)

Holy Fervor

Colin Alston had some interesting things to say about "religious" technology debates and managed to nicely support Twisted and Divmod's Nevow at the same time. I'm saddened when people try to debate with us about the "right way" to do network programming (right now, everyone's asking how we compare to Erlang). This isn't a jihad, folks. Colin said it very well:
Why is it though that people become so fanatical about a particular framework or language? At the end of the day our choices of language and framework should be based on merit and not on emotion.
Colin also highlighted a few of the many reasons I like to use Twisted and Nevow:
Once you can break this mould, the endless possibilities of web applications become far more apparent. Twisted and Nevow gives you a clean web server tailored for your application, and you can keep your own state information and event control without needing the client to hit on special pages or abusing AJAX to poll resources.
Scaling

Next up is Glyph's absolutely fantastic essay on scaling with Twisted and Mantissa. JP and I have actually been talking to him about this stuff in great detail lately, due to the messaging work that Glyph is doing. It's a delight that Glyph took the time to put his thoughts and analysis on "paper." There's too much good stuff in there to quote it; you'll have to read the whole thing.

Intermediate to Advanced

Glyph gets the spotlight again with his response to a recent critique of Twisted by a programmer who prefers to write threaded applications. This is a must-read for intermediate to advanced Twisters.

In a similar vein, folks wanting to push their Twisted skills further and do some integration would do well to read this post, which got upmodded at reddit.com last week. It covers some basic advice on creating Twisted-ready async code.

More Blog Bits

JP sketched out a quick prototype of TCP in the browser using Nevow/Athena. The PyPy team has Nevow running on PyPy now! Prasanna Gautam has a quick example of sending binary data over XML-RPC using Twisted. Jack Moffitt of Chesspark fame blogged about their new chat product (based on Twisted) called speeqe. Jack gives us another node in his earlier Firefox GTD blog post. There was a post by Duncan covering some examples of "batching" deferreds in Twisted. Glyph posted an interesting comment on Steve Holden's blog. And finally, Jonathan Lange gives a shout out to the crew on his post about code reviews.