Chuck Burgess [2007-09-11 02:25 UTC] Looks like a nice concise package to do the straightforward job of setting & getting TinyURLs.
Your package.xml says BSD license, though all the code files show the PHP license, both in @license tag and in license text block.
Granted, this is only the second proposal I've ever looked at, but I would have expected to see the PEAR/Exception be required by the Exception class file, and the regular class file in turn requiring the TinyURL_Exception file. That's just an observation... I don't know what the coding standards say on that.
I can't seem to get the PHPTs to pass. I have Validate 0.7.0 and the curl extension installed, using PHP 5.2.4.
Till Klampaeckel [2007-09-11 02:45 UTC] First off, good job. You kind of beat me to it.
It's very simple and easy to use, the only thing I want to add (on top of what Chuck said), is that I noticed that when I create a tinyurl and I try to look the same one up, it says - "no redirection found".
How long does it take until urls create through the API become available through the API?
Helgi Þormar Þorbjörnsson [2007-09-11 03:02 UTC] Looks straight forward enough, small, simple, elegant.
Haven't tested it yet but I will.
One observation tho, do you plan on using curl if this gets accepted or change to HTTP_Request2 when (if ?! :P) it comes out.
Joe Stump [2007-09-11 03:33 UTC] 1.) http://bugs.joestump.net/trac.cgi/pear/ticket/8 (Addresses all of the comments so far)
2.) I'll be moving to HTTP_Request2 for all of my Services_* packages once it's released. Until then I'm stuck with curl.
Travis Swicegood [2007-09-11 12:28 UTC] Couple of thoughts:
1) What about those of us who like to instantiate everything? :-) Would a Services_TinyURL::singleton/getInstance() be appropriate, or am I the only one that would want to instantiate this object?
2) I think there's a typo on the pattern of the first preg_match() of lookup(). "[a-z0-91]" - the "1" is redundant, no?
3) Why use preg_match_all() at the end of lookup()?
Joshua Eichorn [2007-09-11 16:07 UTC] Http_Request2 is currently only targeted at pear2 ie PEAR2_Http_Request. If your interested in helping to make a Http_Request2 release happen let me know. The code is ready for a alpha release at this point.
Chuck Burgess [2007-09-12 00:40 UTC] All my earlier comments look to be handled :)
I did get the tests to pass via "pear run-tests -p", so I'm good.
Joe Stump [2007-09-13 00:37 UTC] 1.) http://bugs.joestump.net/trac.cgi/pear/ticket/9 (Fixes all problems up to this point)
2.) Change to a class that can be instantiated.
|
