Spec URL: http://nb.fedorapeople.org/horde-reviews/php-horde-Horde-Cache.spec SRPM URL: http://nb.fedorapeople.org/horde-reviews/php-horde-Horde-Cache-1.0.4-1.fc16.src.rpm Description: This package provides a simple, functional caching API, with the option to store the cached data on the filesystem, in one of the PHP opcode cache systems (APC, eAcclerator, XCache, or Zend Performance Suite's content cache), memcached, or an SQL table.
Updated Spec URL: http://nb.fedorapeople.org/horde-reviews/php-horde-Horde-Cache.spec SRPM URL: http://nb.fedorapeople.org/horde-reviews/php-horde-Horde-Cache-1.0.5-1.fc16.src.rpm
I will review this package
I believe all of the normal blockers for the php-horde-Horde-* packages are fixed with this package.
[MUST] "Requires: php-common >= 5.2.0" to satisfy package.xml [MUST] Change Requires: php-pear(pear.horde.org/Horde_Util) <= 2.0.0 to Requires: php-pear(pear.horde.org/Horde_Util) < 2.0.0 to satisfy package.xml [OPTIONAL] For readability, you may want to group your Build* and Requires* [OPTIONAL] I'm not sure what the exact Fedora packaging guidelines are for this, but I prefer to be very verbose and would list both the min and max requirements instead of just max (even though you know you don't have < 1.0.0 in Fedora): Requires: php-pear(pear.horde.org/PACKAGE) >= 1.0.0 Requires: php-pear(pear.horde.org/PACKAGE) < 2.0.0 [OPTIONAL] You may want to consider adding requires for the following optional dependencies (from package.xml): * php-pear(pear.horde.org/Horde_Db) * php-pear(pear.horde.org/Horde_Log) * php-pear(pear.horde.org/Horde_Memcache) -- be careful with this one as I don't know whether this would cause issues in certain environments like APC does (which is also an optional requirement of this package) [OPTIONAL] phpci: You may want to consider adding the following requires: * php-date * php-hash * php-spl * php-pecl(LZF) >= 1.5.2 --- NOTE: package.xml lists the hash extension as required, but it is a virtual package of php-common (as php-hash) so it would automatically be installed. You may want to consider adding the requirement to be thorough and prevent any future issue of there is PHP package changes. --- NOTE: You may want to have upstream add the additional optional extensions (LZF is already listed in package.xml as optional) [OPTIONAL] You may want to s/pear.horde.org/%{pear_channel}/ and then add "%global pear_channel pear.horde.org"
(In reply to comment #4) > [OPTIONAL] I'm not sure what the exact Fedora packaging guidelines are for > this, but I prefer to be very verbose and would list both the min and max > requirements instead of just max (even though you know you don't have < > 1.0.0 in Fedora): > Requires: php-pear(pear.horde.org/PACKAGE) >= 1.0.0 > Requires: php-pear(pear.horde.org/PACKAGE) < 2.0.0 Per https://bugzilla.redhat.com/show_bug.cgi?id=785446#c6, ignore this
Updated Spec URL: http://nb.fedorapeople.org/horde-reviews/php-horde-Horde-Cache.spec SRPM URL: http://nb.fedorapeople.org/horde-reviews/php-horde-Horde-Cache-2.0.1-1.fc17.src.rpm
Please remove APC from requires. It is an optional require and end-users should not be forced to install APC if they are using one of the other opcode caches.
Updated Spec URL: http://nb.fedorapeople.org/horde-reviews/php-horde-Horde-Cache.spec SRPM URL: http://nb.fedorapeople.org/horde-reviews/php-horde-Horde-Cache-2.0.1-2.fc17.src.rpm
Created attachment 666397 [details] phpci.log
Created attachment 666398 [details] php-horde-Horde-Cache-review.txt Generated by fedora-review 0.3.1 (b71abc1) last change: 2012-10-16 Buildroot used: fedora-rawhide-x86_64 Command line :/usr/bin/fedora-review -b 785450 --mock-config fedora-rawhide-x86_64
===== COULD items ===== * "s/pear.horde.org/%{pear_channel}/" and add "%global pear_channel pear.horde.org" * Add a note in %description letting end-users know that they are responsible for installing whichever support opcode cache they would like. No blockers. ===== APPROVED =====
New Package SCM Request ======================= Package Name: php-horde-Horde-Cache Short Description: This package provides a simple, functional caching API for Horde Owners: nb Branches: f17 f18 el6 InitialCC: remi
Git done (by process-git-requests).