Re: services_libravatar caching

Hello Stelian, > Changes are now in my fork, please have a look. > >> 3. Please move DriverCacheLite.php to Driver/CacheLite and rename > >> the class accordingly. 1. I'm a bit disappointed that you don't know about the PEAR Class-to-filename conventions. They are even publicly accepted via PSR-0: > $filename = str_replace($classname, '_', '/') . '.php' So Services_Libarvatar/Cache_Driver should be in Services/Libravatar/Cache/Driver.php For including the files, you assume that the file with its full is in include path: > require_once 'Services/Libravatar/Driver.php'; No __DIR__ and "../" at all. Btw, all classes in that package need to be prefixed with Services_Libravatar - so 2. There should be no default driver set, or at least a "null" driver that does no caching at all. Currently Cache_Lite is required; something that I would not want at all. 3. protected $dnsCache has "@var mixed". Why mixed? Shouldn't all drivers implement the Services_Libravatar_Cache_Driver interface Or is null allowed? Replace "mixed" with "Services_Libravatar_Cache_Driver" 4. "@param int $time - length in seconds". Please remove the "-", it's not necessary and not used anywhere else. 5. "@return bool;" Please describe what the return value means, e.g. "true if saving succeeded, false if not" 6. "@return null;" This needs to be "@return void", to indicate no return value. You also do not need to add "return null;" to that code; PHP does that automatically anyway. 7. "Class available since Release @package_version@" This would always update the package version, which is not what we want :) The cache driver is a new feature, so the next Services_Libravatar version containing it will be 0.3.0. Set this version hard in there. 8. I'd like to be able to change the settings of the Cache_Lite object. So please either allow passing a Cache_Lite object to the Driver constructor, or make a getCacheLite() method in that driver. -- Regards/Mit freundlichen Grüßen Christian Weiske -=≡ Geeking around in the name of science since 1982 ≡=-

Attachment: [application/pgp-signature] signature.asc