https://bugzilla.redhat.com/show_bug.cgi?id=785465 --- Comment #3 from Shawn Iwinski <shawn.iwinski@xxxxxxxxx> --- *** [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. -- You are receiving this mail because: You are on the CC list for the bug. _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review