[PEPr] Comment on Networking::Net_SSH2

From: Date: Wed, 09 Sep 2009 19:45:28 +0000
Subject: [PEPr] Comment on Networking::Net_SSH2
References: 1  Groups: php.pear.dev 
Request: Send a blank email to pear-dev+get-52796@lists.php.net to get a copy of this message
Hey Luca, first off, I'm too looking forward to this code in PEAR. I also briefly looked at your code and here are some suggestions: * Move constants into the Net_SSH2 class. * & is not necessary in PHP5. * Always work with visibility (public, static, etc.) on methods (e.g. the factory). * Add optional dependency on PEAR::System and ext/proc to package.xml. (PEAR::File should be optional as well). * Maybe briefly test for File and System, before use (maybe in __construct()). Or introduce a small autoload, or require_once the files in between file and class comment. * Maybe wrap the options into a Net_SSH2::$data array, instead of $this->$key = $val, $this->data[$key] = $val (in the drivers __construct()). * In Net_SSH2_LibSSH2 you could support the .dll version (on Windows), you could also attempt loading the php_ssh2.dll/ssh2.so before you error out. * Document thrown Exceptions with @throws. * Instead of die() in your tests, please use $this->markTestIncomplete() or $this->markTestSkipped(). * Also, for some tests, take a look at $this->setExpectedException(). Anyway, your code is great regardless of what I find. Especially nice that you put a lot of effort into your test suite. :) Cheers, Till -- http://pear.php.net/pepr/pepr-proposal-show.php?id=586

« previous php.pear.dev (#52796) next »