[PEPr] +1 for Networking::Net_WebFinger
| From: | Till Klampaeckel | Date: | Fri, 17 Feb 2012 14:19:24 +0000 |
| Subject: | [PEPr] +1 for Networking::Net_WebFinger | ||
| References: | 1 | Groups: | php.pear.dev |
| Request: | Send a blank email to pear-dev+get-54826@lists.php.net to get a copy of this message | ||
Till Klampaeckel (http://pear.php.net/user/till) has voted +1 on the proposal for
Networking::Net_WebFinger.
Proposal information:
http://pear.php.net/pepr/pepr-proposal-show.php?id=675
Vote information:
http://pear.php.net/pepr/pepr-vote-show.php?id=675&handle=till
Comment:
Overall really well done.
1) What I'm not so sure about is the request code in the Net_WebFinger
class. Some of the methods seem long and a little redundant in terms of
code.
Maybe spin it off into a dedicated request class. I have no idea yet, if
you catch my drift, maybe you can improve this, otherwise I'll take a look
later and will send you a pull request.
2) Net_WebFinger::finger() should IMHO throw an exception if no '@' is
found.
3) I'd see if you can use Net_URL2 (which comes with HTTP_Request2) for the
"isHttps()" check. Speaking of which, is SSL usually a given with these
things or the exception? If most of the resources are non-SSL, I'd run the
non-SSL request first. Otherwise it'll be pretty expensive.
And/Or offer a way to force "non-SSL" lookup (with finger()).
4) I'm not so sure if I like Net_WebFinger_Error. Are all errors usually to
be disregarded? Is that why you don't throw Exceptions? Or is there another
reason I am missing.
5) I'd like to see an autoloader. I'm not sure why you think PEAR1 cannot
have this. Especially with a 5.3 minimum, require_once seems really
outdated and would force me to fork for my own uses to use another
class-loading strategy.
Imho, an autoloader could take care of all that. Enabled by default, and if
people want to run their own, they could disable it. Then there's no need
to strip require_once.
6) Careful on the dependency on Cache. ;) (I think we need to work on
something there and bring the code up to date.)