Spec URL: http://nb.fedorapeople.org/horde-reviews/php-horde-Horde-Auth.spec SRPM URL: http://nb.fedorapeople.org/horde-reviews/php-horde-Horde-Auth-1.4.7-1.fc16.src.rpm Description: The Horde_Auth package provides a common interface into the various backends for the Horde authentication system.
I believe this one is ready to review now.
Updated Spec URL: http://nb.fedorapeople.org/horde-reviews/php-horde-Horde-Auth.spec SRPM URL: http://nb.fedorapeople.org/horde-reviews/php-horde-Horde-Auth-1.4.9-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 [SHOULD] lib/Horde/Auth/Sql.php includes "$now = new Horde_Date(time());". Please verify Horde_date is not a dependency. [SHOULD] %check is present and all tests pass [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_History) * php-pear(pear.horde.org/Horde_Lock) * php-pear(pear.horde.org/Horde_Imap_Client) * php-pear(pear.horde.org/Horde_Kolab_Session) * php-pear(pear.horde.org/Horde_Ldap) * php-pear(pear.horde.org/Horde_Imsp) * php-pear(pear.horde.org/Horde_Http) [OPTIONAL] phpci: You may want to consider adding the following requires: * php-ctype * php-date * php-ftp * php-hash * php-ldap * php-pcre * php-pdo * php-spl --- NOTE: Some of these may be from the optional horde packages mentioned in the previous [OPTIONAL] so you may not need to require them if you do not require those --- NOTE: package.xml specifies "ctype" and "ftp" extensions as optional... you may want to have upstream add the additional optional extensions if they are not already required by the mentioned optional packages (I'm assuming ldap would be required by Horde_Ldap, pdo would be required by Horde_Db, etc). [OPTIONAL] package.xml lists PECL "pam" and "sasl" as optional. These are not packaged in Fedora (at least I couldn't find them). Looking at the PECL site, I'm not sure if those extensions are actively maintained or not though. If they are, you may want to package them and eventually add them as dependencies. [OPTIONAL] You may want to s/pear.horde.org/%{pear_channel}/ and then add "%global pear_channel pear.horde.org"
(In reply to comment #5) > [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-Auth.spec SRPM URL: http://nb.fedorapeople.org/horde-reviews/php-horde-Horde-Auth-2.0.1-1.fc17.src.rpm
Updated Spec URL: http://nb.fedorapeople.org/horde-reviews/php-horde-Horde-Auth.spec SRPM URL: http://nb.fedorapeople.org/horde-reviews/php-horde-Horde-Auth-2.0.2-1.fc17.src.rpm
quick notes: - missing %check - missing %{php_metadir} - must own %{pear_datadir}/Horde_Auth/migration - should use %{pear_name} (and perhaps %{channel_name})
I don't think we need to have Horde_Test in the optional dep (even if available) as this pull a lot of (probably unwanted) dependencies.
Updated Spec URL: http://nb.fedorapeople.org/horde-reviews/php-horde-Horde-Auth.spec SRPM URL: http://nb.fedorapeople.org/horde-reviews/php-horde-Horde-Auth-2.0.3-2.fc17.src.rpm
Shawn, do you plan to review this one ? (the next according to my build order)
(In reply to comment #12) > Shawn, do you plan to review this one ? (the next according to my build > order) I just tried to, but pear.horde.org is unreachable :( I'll release my review in case you can get to it before I do.
Nick -- Without being able to download the upstream source and do an official review, here are some things I see that need to be addressed: * BuildRequires: php-pear ===> php-pear(PEAR) >= 1.7.0 * BuildRequires: php-common >= 5.3.0 * Requires: php-pear(PEAR) >= 1.7.0 Also, I'm guessing that the "LGPLv2+" license listed needs to be changed to "LGPLv2" like a bunch of the other Horde packages.
Updated Spec URL: http://nb.fedorapeople.org/horde-reviews/php-horde-Horde-Auth.spec SRPM URL: http://nb.fedorapeople.org/horde-reviews/php-horde-Horde-Auth-2.0.4-1.fc17.src.rpm
Created attachment 716958 [details] phpci.log phpci 2.14.0
Created attachment 716960 [details] review.txt Generated by fedora-review 0.4.0 (660ce56) last change: 2013-01-29 Buildroot used: fedora-rawhide-x86_64 Command line :/usr/bin/fedora-review -b 785447
[!]: Each %files section contains %defattr if rpm < 4.4 [!]: Requires correct, justified where necessary. Missing from phpci report Requires: php-ctype Requires: php-date Requires: php-ftp Requires: php-hash Requires: php-ldap Requires: php-pcre Requires: php-pdo Requires: php-spl There is a lot of optional dependencies from package.xml. Need to evaluate which ones can be added without breaking the build order. (at least Horde_Db, Horde_Lock)
Actually Updated Spec URL: http://nb.fedorapeople.org/horde-reviews/php-horde-Horde-Auth.spec SRPM URL: http://nb.fedorapeople.org/horde-reviews/php-horde-Horde-Auth-2.0.4-2.fc17.src.rpm
-%defattr(-,root,root,-) [x]: Each %files section contains %defattr if rpm < 4.4 +Requires: php-ctype php-date php-ftp php-hash php-ldap +Requires: php-pcre php-pdo php-spl +Requires: php-pear(%{pear_channel}/Horde_Db) >= 2.0.0 +Conflicts: php-pear(%{pear_channel}/Horde_Db) >= 3.0.0 +Requires: php-pear(%{pear_channel}/Horde_Lock) >= 2.0.0 +Conflicts: php-pear(%{pear_channel}/Horde_Lock) >= 3.0.0 +Requires: php-pear(%{pear_channel}/Horde_Http) >= 2.0.0 +Conflicts: php-pear(%{pear_channel}/Horde_Http) >= 3.0.0 +Requires: php-pear(%{pear_channel}/Horde_Ldap) >= 2.0.0 +Conflicts: php-pear(%{pear_channel}/Horde_Ldap) >= 3.0.0 [x]: Requires correct, justified where necessary. No Blocker, all issues fixed. === APPROVED ===
New Package SCM Request ======================= Package Name: php-horde-Horde-Auth Short Description: Horde Authentication API Owners: nb remi Branches: el6 f18 f19 InitialCC:
Git done (by process-git-requests).
php-horde-Horde-Auth-2.0.4-2.fc18 has been submitted as an update for Fedora 18. https://admin.fedoraproject.org/updates/php-horde-Horde-Auth-2.0.4-2.fc18
php-horde-Horde-Auth-2.0.4-2.el6 has been submitted as an update for Fedora EPEL 6. https://admin.fedoraproject.org/updates/php-horde-Horde-Auth-2.0.4-2.el6
php-horde-Horde-Auth-2.0.4-2.el6 has been pushed to the Fedora EPEL 6 stable repository.
php-horde-Horde-Auth-2.0.4-2.fc18 has been pushed to the Fedora 18 stable repository.