[PEPr] +1 for Web Services::Services_ShortURL
| From: | Michael Gauthier | Date: | Wed, 20 May 2009 21:40:23 +0000 |
| Subject: | [PEPr] +1 for Web Services::Services_ShortURL | ||
| References: | 1 | Groups: | php.pear.dev |
| Request: | Send a blank email to pear-dev+get-51840@lists.php.net to get a copy of this message | ||
Michael Gauthier (http://pear.php.net/user/gauthierm) has voted +1 on the proposal for Web
Services::Services_ShortURL.
Proposal information:
http://pear.php.net/pepr/pepr-proposal-show.php?id=598
Vote information:
http://pear.php.net/pepr/pepr-vote-show.php?id=598&handle=gauthierm
Comment:
Well written. For the convenience of others, this package is BSD licensed,
not PHP licensed as the proposal indicates.
Couple of feedback points:
1.) cURL extension is no required or optional in package.xml but
HTTP_Request2 is created with cURL by default.
2.) Why explicitly use cURL by default? Why not let HTTP_Request2 decide
that one?
3.) The accept() method is confusing to me. Why not setRequest(). Also,
you could use type hinting here rather than checking the parameter type
manually. The only reasons I can think of for doing it this way are if you
are implementing a generic interface but that does not seem to be the
case.
4.) License tag links shouldn't use tinyurl because links could disappear
in the unlikely even that tinyurl disappears.
5.) That's all I've got. Looks great.