Vote Details for "Net_FastCGI" by gauthierm

» Details
Voter: Michael Gauthier Vote: +1 (conditional) Reviews: Deep source review
» Comment
Code is well written and documented. I have a few comments, suggestions and questions. 1.) Is this a replacement or addition to compiling PHP with --enable-fastcgi? Does it provide benefits above and beyond the extension? See http://php.net/manual/en/configure.php#configure.enable-fastcgi 2.) Since this is a fairly complicated package, end user documentation is necessary. Include: - What Net_FastCGI does, - How you configure a server to use it. 3.) Protected properties need documentation. 4.) Documentation should be written in third person declarative. Use "Stops the server." instead of "Stop the server". See http://pear.php.net/manual/en/standards.sample.php 5.) The package2.xml should have a dependency on posix because the pnctl functions won't work on windows. It should also have a dependency on the PHP pnctl extension. 6.) The @see tag only links to inline documentation. You'll need to use a @link tag to link to php functions in documentation. For example: "@link http://foo/bar bar" will work. 7.) A license other then the PHP license would be nice but is not required. The PHP license is incompatible with a lot of other open-source licenses. 8.) Auto-detection of the php-cgi binary location would be nice. You could check a list of common locations for various distros (Ubuntu, Fedora, OSX, built from source) and if it's not found and is not specified, throw an exception. See Crypt_GPG for an example. 9.) Environment variables could default to $_ENV if not specified. 10.) Specific exception classes are needed. Net_FastCGI_InvalidSocketException, etc. 11.) Linking to a RFC or specification in the documentation of packing and unpacking of data would be useful. 12.) Where strlen() is used, consider using a method that handles the case when mbstring function overloading is enabled. See Crypt_GPG_Engine::_byteLength() for an example. Same goes for substr(). 13.) s/readden/read in documentation. 14.) Have you tested sending large responses and receiving large requests? This might turn up bugs in the process control and IO-streaming code.