[PEPr] Comment on Networking::Net_SSH2
| From: | Till Klampaeckel | 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