Chuck Burgess [2008-11-15 21:57 UTC] I'm afraid that having only a tarball download for seeing the source code and having only CHM for documentation will seriously limit who can/will review your proposal. At a minimum, I'd suggest running phpDocumentor on your code and host the docs somewhere... including the --sourcecode runtime option will result in the docs containing highlighted sourcecode views, though actually making .phps files available would be better.
Ken Guest [2008-11-18 13:37 UTC] there are some slight inconsistencies as some of the messages being written to the log include timestamps and other don't...
other than that it looks good (there are a few phpcs/coding standards errors and warnings though).
Michael Gauthier [2011-01-30 22:07 UTC] 1.) Use scope when declaring functions (public/private/protected)
2.) Making the package version 1.0 is premature. Usually packages start at 0.1 and go through some PAI refinement before reaching 1.0. When your package reaches 1.0 you must keep the API backwards compatible from that point on.
3.) Use class constants instead of define().
4.) Directory structure should follow pear conventions. Your files are already named correctly but directory structure should reflect the filenames. The _ in filenames translates to / in the path.
5.) Run phpcs on the code. Lots of lines are longer than the maximum allowed length.
6.) sockets is a required extension for this package and should be listed in the package.xml file.
7.) Package description should give some idea of what SNMP is. I have no idea :)
Joselyn Camero [2011-03-15 15:08 UTC] Hi,
I will be working in these recommendations as soon as posible.
Thanks :)
Damien Bezborodov [2011-03-18 02:02 UTC] All looks great to me.
I would personally amend point 13 a little bit. Dependency injection is great for unit testing, but can be a bit of an inconvenience when using the code in the real world. Instead, make it optional.
//Bad
function bar() {
$bar = new Bar();
return $bar->doSomething();
}
//Good
function bar(Bar $bar = null) {
if (!$bar instanceof Bar) {
$bar = new Bar();
}
return $bar->doSomething();
}
//Good
class X {
function __construct($bar = null) {
if (!$bar instanceof Bar) {
$bar = new Bar();
}
$this->bar = $bar;
}
function bar() {
return $this->bar->doSomething();
}
}
|
