Re: Net_Vpopmaild
| From: | Joe Stump | Date: | Fri, 13 Jul 2007 08:51:44 +0000 |
| Subject: | Re: Net_Vpopmaild | ||
| References: | 1 | Groups: | php.pear.dev |
| Request: | Send a blank email to pear-dev+get-47457@lists.php.net to get a copy of this message | ||
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Some initial feedback:
1.) It seems everything is public. I'd think $log, for instance, could be private. I don't see a reason why anything would need to log outside of this package. Additionally, it might make more sense to implement an accept() method so that I can pass my own non-file Log object to Net_Vpopmail for logging.
2.) All if/else, etc. must have { } e.g. one line if statements don't conform to PEAR CS. It must be:
if (true) {
return true;} 3.) I'd highly recommend using preg_* instead of ereg. I'm not sure if this is explicitly stated in PEAR CS, but php.net widely considers ereg to be outdated. 4.) PHP5 packages cannot use PEAR_Error and must use exceptions. Create Net_Vpopmail_Exception extending PEAR_Exception and throw that instead of PEAR_Error. 5.) I'm fairly sure Net_Socket and the Mail package you use are PHP4. PHP5 PEAR packages cannot rely on PHP4 packages per the RFC's. 6.) In setLimits() I'd make those arrays static so they aren't created in memory for each time the function is called. 7.) Instead of validEmailAddress() maybe use Validate::email() ? Not sure if that's RFC compliant though. 8.) Nitpicky, but I don't see any .phpt or unit tests. For a first time package, though, this looks good and definitely serves an awesome purpose. Thanks! - --Joe On Jul 12, 2007, at 9:47 PM, Bill Shupp wrote:
Hi folks, I'm working on my first PEAR package, Net_Vpopmaild, which I plan to submit for consideration. But before I do, I could use some feedback on it, as I suspect it'll need some work (i.e. I'm wondering if it should be broken into several classes or not, and the error handling might need improvement). The API can be viewed here: http://shupp.org/Net_Vpopmaild/ Background: Vpopmail is a qmail add-on package for managing virtual domains. vpopmaild is a new daemon for managing vpopmail over a tcp connection. Net_Vpopmaild uses Net_Socket, Mail_RFC822, and Log. Status: Most of the functionality works, including authentication, adding/removing users, domains, forwards, and autoresponders. I've only started the ezmlm support, though. Thanks! Bill Shupp --PEAR Development Mailing List (http://pear.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php-----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.5 (Darwin) iD8DBQFGlz0jh0MUGpYY9OQRAukNAJ4hHXCFzpNmn/y3uXEQPwhHM61kLgCgw69B ifQLGRKooZVgjMeMfOa2AyI= =f6Uq -----END PGP SIGNATURE-----