[prev in list] [next in list] [prev in thread] [next in thread] 

List:       pear-dev
Subject:    [PEAR-DEV] [PEPr] +1 for PHP::Mailman
From:       "Till Klampaeckel" <till () php ! net>
Date:       2011-09-17 1:21:22
Message-ID: 20110917022217.1F469193A42 () euk1 ! php ! net
[Download RAW message or body]

Till Klampaeckel (http://pear.php.net/user/till) has voted +1 on the proposal for \
PHP::Mailman.

Proposal information:
http://pear.php.net/pepr/pepr-proposal-show.php?id=665
Vote information:
http://pear.php.net/pepr/pepr-vote-show.php?id=665&handle=till

This vote is conditional. The condition is:

What Christian and Ken said.

In addition:

1. Can you have an empty password on a mailman installation? If not, an exception \
should be thrown -- IMHO the error needs to be explicit. In most of these cases \
is_string is nice, but kinda meaningless. I'd check for empty() if something is \
required.

I'd also leave out a default (in __construct()) if something is required. 

Same for admin url and listname. Asking the object for an error shouldn't be \
necessary since these are the most basic requirements for the class to work.

If you disagree, please let me know why. Maybe I'm wrong, or don't know the setup \
well enough.

2. I'd also like to emphasize the unittest thing.

The following are minors (aka nice to see in the next release, etc.):

3. Speaking of 'hasErrors()' 1 is not true and 0 is not false.

4. @version tags are mixed up.

5. I like set-methods with a fluent interface. E.g. "return $this;" -- makes the \
calls chainable. If there's an error, just throw an exception. Makes the API easier \
and cleaner (IMHO).

6. Your checks in setReq() are redundant -- if it wasn't an instance of \
HTTP_Request2, it would never get so far since the type hint would throw it off.

7. List could be its own object Service_Mailman_List with methods to \
subscribe/unsubscribe, list members, etc.. But that's just a suggestion not mandatory \
to make my vote +1.


Thanks again for proposing this code and thanks for incorporating my previous \
suggestions! :-)

-- 
PEAR Development Mailing List (http://pear.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php


[prev in list] [next in list] [prev in thread] [next in thread]