Spec URL: http://nb.fedorapeople.org/horde-reviews/php-horde-Horde-Group.spec SRPM URL: http://nb.fedorapeople.org/horde-reviews/php-horde-Horde-Group-1.0.4-1.fc16.src.rpm Description: Package for managing and accessing the Horde groups system.
Updated Spec URL: http://nb.fedorapeople.org/horde-reviews/php-horde-Horde-Group.spec SRPM URL: http://nb.fedorapeople.org/horde-reviews/php-horde-Horde-Group-1.0.5-1.fc16.src.rpm
I will review this package
*** [MUST] Please check the license. I see LGPLv2 (from source) but I do not see LGPLv2+ (from spec), i.e. "LGPLv2 or later", mentioned anywhere. *** [SHOULD] It would help readability to group your Build* and Requires* statements together instead of mixing them with each other and your Provides and Conflicts statements *** [MUST] Your BuildRequires should be BuildRequires: php-pear(PEAR) >= 1.7.0 instead of BuildRequires: php-pear >= 1:1.4.9-1.2 (this would match what you have in bug 785606 and satisfy this package's package.xml dependency) *** [MUST] Add "Requires: php-common >= 5.2.0" to satisfy package.xml requirement *** [SHOULD] phpci results: For completeness (and to prevent any future packaging issues due to PHP package changes) you may wish to require the virtual package "php-pcre". *** [SHOULD] package.xml lists dependencies of Horde_Util and Horde_Exception >= 1.0.0 and < 2.0.0. You already have requires for < 2.0.0. I'm not sure if it's required, but I like to be verbose so I would suggest adding the >= 1.0.0 requires even though it appears all packages in Fedora satisfy this requirement already. *** [SHOULD] Please consider adding requires for the optional dependencies listed in package.xml: Horde_Db >= 1.0.0 < 2.0.0 Horde_Ldap >= 1.0.0 < 2.0.0 (if you do added requires for these optional dependencies, please update this ticket's "Depends On") *** [SHOULD] Please consider adding a note for optional dependencies "Horde_Test", "Horde_Log" and "php-pdo" (perhaps in %description for end users) and note what they are needed for (it appears all those are requirements of the tests?). *** [MUST] Your provides should be Provides: php-pear(pear.horde.org/%{pear_name}) = %{version} instead of Provides: php-pear(%{pear_name}) = %{version} *** [SHOULD; see https://bugzilla.redhat.com/show_bug.cgi?id=817303#c5] About [ -f package2.xml ] || mv package.xml package2.xml mv package2.xml %{pear_name}-%{version}/%{name}.xml This is a old hack (yes it works) to ensure than package v2 is used. I often use # package.xml is version 2.0 mv package.xml %{pear_name}-%{version}/%{name}.xml This is not an issue, but makes the spec simpler to read. *** [MUST] In %files, use %{pear_name} where you can. *** [SHOULD] It may be beneficial to use "%global pear_channel pear.horde.org" and then use "%{pear_channel}" where you can.
Please update to 2.0.1
Updated Spec URL: http://nb.fedorapeople.org/horde-reviews/php-horde-Horde-Group.spec SRPM URL: http://nb.fedorapeople.org/horde-reviews/php-horde-Horde-Group-2.0.1-2.fc17.src.rpm
Created attachment 696065 [details] phpci.log
Created attachment 696066 [details] 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 -m review -b 785465
[!]: License field in the package spec file matches the actual license. LGPLv2 (not LGPLv2+) [!]: Package installs properly. Note: Installation errors (see attachment) Wait for dependency before import (Horde_Db) [!]: Requires correct, justified where necessary. From package.xml Requires: php-pear(PEAR) >= 1.7.0 And for consistency BuildRequires: php-common >= 5.3.0 BuildRequires: php-pear(PEAR) >= 1.7.0 Optional requires to consider Horde_Ldap (I think we should add this one) pgp-pdo, php-mysql, php-mysqli (will be pulled by Horde_Db) php-ldap (will be pulled by Horde_Ldap)
Updated Spec URL: http://nb.fedorapeople.org/horde-reviews/php-horde-Horde-Group.spec SRPM URL: http://nb.fedorapeople.org/horde-reviews/php-horde-Horde-Group-2.0.2-1.fc17.src.rpm
--- php-horde-Horde-Group.spec.0 2013-02-11 13:21:24.000000000 +0100 +++ php-horde-Horde-Group.spec 2013-03-20 21:26:09.000000000 +0100 @@ -4,17 +4,18 @@ %global pear_channel pear.horde.org Name: php-horde-Horde-Group -Version: 2.0.1 -Release: 2%{?dist} +Version: 2.0.2 +Release: 1%{?dist} Summary: Horde User Groups System Group: Development/Libraries -License: LGPLv2+ +License: LGPLv2 URL: http://pear.horde.org Source0: http://%{pear_channel}/get/%{pear_name}-%{version}.tgz BuildArch: noarch -BuildRequires: php-pear +BuildRequires: php-pear(PEAR) >= 1.7.0 +BuildRequires: php-common >= 5.3.0 BuildRequires: php-channel(%{pear_channel}) # To run unit tests BuildRequires: php-pear(%{pear_channel}/Horde_Test) >= 2.1.0 @@ -23,7 +24,8 @@ Requires(post): %{__pear} Requires(postun): %{__pear} Requires: php-common >= 5.3.0 -Requires: php-pcre +Requires: php-pear(PEAR) >= 1.7.0 +Requires: php-pcre php-pdo php-ldap Requires: php-channel(%{pear_channel}) Requires: php-pear(%{pear_channel}/Horde_Exception) >= 2.0.0 Conflicts: php-pear(%{pear_channel}/Horde_Exception) >= 3.0.0 @@ -32,6 +34,8 @@ # Optionnal 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_Ldap) >= 2.0.0 +Conflicts: php-pear(%{pear_channel}/Horde_Ldap) >= 3.0.0 Provides: php-pear(%{pear_channel}/%{pear_name}) = %{version} @@ -94,16 +98,19 @@ %changelog +* Wed Mar 20 2013 Nick Bebout <nb> - 2.0.2-1 +- Update for review + * Tue Feb 5 2013 Nick Bebout <nb> - 2.0.1-2 - Remove BuildRoot, Change php(language) to php-common -* Mon Nov 19 2012 Remi Collet <RPMS> - 2.0.1-1 +* Mon Nov 19 2012 Remi Collet <remi> - 2.0.1-1 - Update to 2.0.1 for remi repo -* Fri Nov 2 2012 Remi Collet <RPMS> - 2.0.0-2 +* Fri Nov 2 2012 Remi Collet <remi> - 2.0.0-2 - fix provides (missing channel) -* Fri Nov 2 2012 Remi Collet <RPMS> - 2.0.0-1 +* Fri Nov 2 2012 Remi Collet <remi> - 2.0.0-1 - Update to 2.0.0 for remi repo * Thu Jun 21 2012 Nick Bebout <nb> - 1.0.5-1 All issues fixed No blocker === APPROVED ===
New Package SCM Request ======================= Package Name: php-horde-Horde-Group Short Description: Package for managing and accessing the Horde groups system Owners: nb remi Branches: el6 f18 f19 InitialCC:
Git done (by process-git-requests).
php-horde-Horde-Db-2.0.2-1.fc18, php-horde-Horde-Group-2.0.2-1.fc18, php-horde-Horde-LoginTasks-2.0.2-4.fc18, php-horde-Horde-Stream-Filter-2.0.1-4.fc18, php-horde-Horde-Token-2.0.3-3.fc18 has been submitted as an update for Fedora 18. https://admin.fedoraproject.org/updates/php-horde-Horde-Db-2.0.2-1.fc18,php-horde-Horde-Group-2.0.2-1.fc18,php-horde-Horde-LoginTasks-2.0.2-4.fc18,php-horde-Horde-Stream-Filter-2.0.1-4.fc18,php-horde-Horde-Token-2.0.3-3.fc18
php-horde-Horde-Db-2.0.2-1.fc18, php-horde-Horde-Group-2.0.2-1.fc18, php-horde-Horde-LoginTasks-2.0.2-4.fc18, php-horde-Horde-Stream-Filter-2.0.1-4.fc18, php-horde-Horde-Token-2.0.3-3.fc18 has been pushed to the Fedora 18 testing repository.
php-horde-Horde-Db-2.0.2-1.fc18, php-horde-Horde-Group-2.0.2-1.fc18, php-horde-Horde-LoginTasks-2.0.2-4.fc18, php-horde-Horde-Stream-Filter-2.0.1-4.fc18, php-horde-Horde-Token-2.0.3-3.fc18 has been pushed to the Fedora 18 stable repository.