Bug 785450 - (Horde_Cache) Review Request: php-horde-Horde-Cache - Horde Caching API
Review Request: php-horde-Horde-Cache - Horde Caching API
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Shawn Iwinski
Fedora Extras Quality Assurance
:
Depends On: horde-channel Horde_Exception Horde_Util
Blocks:
  Show dependency treegraph
 
Reported: 2012-01-28 20:15 EST by Nick Bebout
Modified: 2013-03-21 11:51 EDT (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2013-01-11 22:10:28 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
shawn: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)
phpci.log (14.39 KB, text/x-log)
2012-12-19 17:07 EST, Shawn Iwinski
no flags Details
php-horde-Horde-Cache-review.txt (7.52 KB, text/plain)
2012-12-19 17:08 EST, Shawn Iwinski
no flags Details

  None (edit)
Description Nick Bebout 2012-01-28 20:15:53 EST
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.
Comment 2 Shawn Iwinski 2012-07-04 19:30:15 EDT
I will review this package
Comment 3 Nick Bebout 2012-07-19 12:49:16 EDT
I believe all of the normal blockers for the php-horde-Horde-* packages are fixed with this package.
Comment 4 Shawn Iwinski 2012-07-21 15:32:26 EDT
[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"
Comment 5 Shawn Iwinski 2012-07-21 17:20:43 EDT
(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
Comment 7 Shawn Iwinski 2012-12-19 13:52:52 EST
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.
Comment 9 Shawn Iwinski 2012-12-19 17:07:44 EST
Created attachment 666397 [details]
phpci.log
Comment 10 Shawn Iwinski 2012-12-19 17:08:34 EST
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
Comment 11 Shawn Iwinski 2012-12-19 17:11:24 EST
===== 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 =====
Comment 12 Nick Bebout 2012-12-20 16:55:15 EST
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
Comment 13 Gwyn Ciesla 2012-12-21 06:44:34 EST
Git done (by process-git-requests).

Note You need to log in before you can comment on or make changes to this bug.