[PEPr] Comment on PHP::Mailman

Hi James. Here are my impressions from an initial overview of the source: For the most part, the code itself seems fine. The only notable exception is $invite at the subscribe() method. Why integer? Why not accept booleans, and conver the value to integer? Also, for the sake of performance, consider getting rid of the _fetch() method. If all you do in it is file_get_contents, you might as well do it without the overhead of a method call. But those are minor issues... the bigger problem IMHO is in the docs. You should keep in mind that tools like PhpDocumentor or similar are going to be generating documentation out of them which would be presented differently, and would not include any source code. e.g. in the constructor you have /** * The class constructor * * @param string $adminurl Sets the class variable * @param string $list Sets the class variable * @param string $adminpw Sets the class variable * * @return void */ And all I'm thinking is "what class variable? What is it for?". Your default values are making things even more misleading. Unless I had read the docs on the class variables, I wouldn't have realized that $list and $adminpw are supposed to be strings ("false" leads me to think those are booleans). Also, avoid relying on any order. Use links instead. e.g. instead of "Set digest (you have to first subscribe them using URL above, then set digest)" use "Set digest. Note that the $email needs to be subsribed first, e.g. by using the {@link subsribe()} method." -- http://pear.php.net/pepr/pepr-proposal-show.php?id=665