Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
Summary: Review Request: php-horde-Horde-Auth - Horde Authentication API
https://bugzilla.redhat.com/show_bug.cgi?id=785447
Summary: Review Request: php-horde-Horde-Auth - Horde Authentication API Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: medium Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: nb@fedoraproject.org QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, package-review@lists.fedoraproject.org Classification: Fedora Story Points: --- Type: --- Regression: --- Mount Type: --- Documentation: ---
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.s... Description: The Horde_Auth package provides a common interface into the various backends for the Horde authentication system.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=785447
Nick Bebout nb@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Depends on| |785424
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=785447
Nick Bebout nb@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Depends on| |785436(horde-exception), | |785439(horde-util) Alias| |horde-auth
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=785447
Bug 785447 depends on bug 785424, which changed state.
Bug 785424 Summary: Review Request: php-channel-horde - Adds pear.horde.org channel to PEAR https://bugzilla.redhat.com/show_bug.cgi?id=785424
What |Old Value |New Value ---------------------------------------------------------------------------- Status|ASSIGNED |MODIFIED Status|MODIFIED |ON_QA Resolution| |ERRATA Status|ON_QA |CLOSED
Bug 785447 depends on bug 785436, which changed state.
Bug 785436 Summary: Review Request: php-horde-Horde-Exception - Horde Exception Handler https://bugzilla.redhat.com/show_bug.cgi?id=785436
What |Old Value |New Value ---------------------------------------------------------------------------- Resolution| |CURRENTRELEASE Status|NEW |CLOSED
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=785447
Bug 785447 depends on bug 785439, which changed state.
Bug 785439 Summary: Review Request: php-horde-Horde-Util - Horde Utility Libraries https://bugzilla.redhat.com/show_bug.cgi?id=785439
What |Old Value |New Value ---------------------------------------------------------------------------- Resolution| |CURRENTRELEASE Status|NEW |CLOSED
https://bugzilla.redhat.com/show_bug.cgi?id=785447
Nick Bebout nb@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |fedora@famillecollet.com
--- Comment #1 from Nick Bebout nb@fedoraproject.org --- I believe this one is ready to review now.
https://bugzilla.redhat.com/show_bug.cgi?id=785447
--- Comment #2 from Nick Bebout nb@fedoraproject.org --- 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.s...
https://bugzilla.redhat.com/show_bug.cgi?id=785447
Shawn Iwinski shawn.iwinski@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED
https://bugzilla.redhat.com/show_bug.cgi?id=785447
Shawn Iwinski shawn.iwinski@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Assignee|nobody@fedoraproject.org |shawn.iwinski@gmail.com
https://bugzilla.redhat.com/show_bug.cgi?id=785447
--- Comment #3 from Shawn Iwinski shawn.iwinski@gmail.com --- I will review this package
https://bugzilla.redhat.com/show_bug.cgi?id=785447
Shawn Iwinski shawn.iwinski@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-review?
https://bugzilla.redhat.com/show_bug.cgi?id=785447
--- Comment #4 from Nick Bebout nb@fedoraproject.org --- I believe all of the normal blockers for the php-horde-Horde-* packages are fixed with this package.
https://bugzilla.redhat.com/show_bug.cgi?id=785447
--- Comment #5 from Shawn Iwinski shawn.iwinski@gmail.com --- [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"
https://bugzilla.redhat.com/show_bug.cgi?id=785447
Shawn Iwinski shawn.iwinski@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Whiteboard| |NotReady
https://bugzilla.redhat.com/show_bug.cgi?id=785447
--- Comment #6 from Shawn Iwinski shawn.iwinski@gmail.com --- (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
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=785447
Nick Bebout nb@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Whiteboard|NotReady |
--- Comment #7 from Nick Bebout nb@fedoraproject.org --- 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.s...
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=785447
--- Comment #8 from Nick Bebout nb@fedoraproject.org --- 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.s...
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=785447
--- Comment #9 from Remi Collet fedora@famillecollet.com --- quick notes: - missing %check - missing %{php_metadir} - must own %{pear_datadir}/Horde_Auth/migration - should use %{pear_name} (and perhaps %{channel_name})
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=785447
--- Comment #10 from Remi Collet fedora@famillecollet.com --- 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.
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=785447
--- Comment #11 from Nick Bebout nb@fedoraproject.org --- 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.s...
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=785447
Remi Collet fedora@famillecollet.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |908329 (Horde_Core)
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=785447
--- Comment #12 from Remi Collet fedora@famillecollet.com --- Shawn, do you plan to review this one ? (the next according to my build order)
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=785447
Remi Collet fedora@famillecollet.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |909706 (Horde_Vfs)
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=785447
Shawn Iwinski shawn.iwinski@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Assignee|shawn.iwinski@gmail.com |nobody@fedoraproject.org Flags|fedora-review? |
--- Comment #13 from Shawn Iwinski shawn.iwinski@gmail.com --- (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.
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=785447
--- Comment #14 from Shawn Iwinski shawn.iwinski@gmail.com --- 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.
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=785447
--- Comment #15 from Nick Bebout nb@fedoraproject.org --- 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.s...
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=785447
Nick Bebout nb@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Alias|horde-auth |Horde_Auth
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=785447
Remi Collet fedora@famillecollet.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Assignee|nobody@fedoraproject.org |fedora@famillecollet.com
--- Comment #16 from Remi Collet fedora@famillecollet.com --- Created attachment 716958 --> https://bugzilla.redhat.com/attachment.cgi?id=716958&action=edit phpci.log
phpci 2.14.0
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=785447
--- Comment #17 from Remi Collet fedora@famillecollet.com --- Created attachment 716960 --> https://bugzilla.redhat.com/attachment.cgi?id=716960&action=edit 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
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=785447
Remi Collet fedora@famillecollet.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-review?
--- Comment #18 from Remi Collet fedora@famillecollet.com --- [!]: 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)
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=785447
Remi Collet fedora@famillecollet.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |929039 (Horde_Kolab_Server)
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=785447
Shawn Iwinski shawn.iwinski@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |927894 (Horde_SyncMl)
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=785447
--- Comment #19 from Nick Bebout nb@fedoraproject.org --- 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.s...
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=785447
--- Comment #20 from Nick Bebout nb@fedoraproject.org --- 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.s...
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=785447
Remi Collet fedora@famillecollet.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? | Flags| |fedora-review+
--- Comment #21 from Remi Collet fedora@famillecollet.com --- -%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 ===
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=785447
Nick Bebout nb@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-cvs?
--- Comment #22 from Nick Bebout nb@fedoraproject.org --- New Package SCM Request ======================= Package Name: php-horde-Horde-Auth Short Description: Horde Authentication API Owners: nb remi Branches: el6 f18 f19 InitialCC:
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=785447
Jon Ciesla limburgher@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-cvs? | Flags| |fedora-cvs+
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=785447
--- Comment #23 from Jon Ciesla limburgher@gmail.com --- Git done (by process-git-requests).
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=785447
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |MODIFIED
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=785447
--- Comment #24 from Fedora Update System updates@fedoraproject.org --- 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
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=785447
--- Comment #25 from Fedora Update System updates@fedoraproject.org --- 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
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=785447
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |CLOSED Resolution|--- |ERRATA Last Closed| |2013-04-04 12:38:21
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=785447
--- Comment #26 from Fedora Update System updates@fedoraproject.org --- php-horde-Horde-Auth-2.0.4-2.el6 has been pushed to the Fedora EPEL 6 stable repository.
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=785447
--- Comment #27 from Fedora Update System updates@fedoraproject.org --- php-horde-Horde-Auth-2.0.4-2.fc18 has been pushed to the Fedora 18 stable repository.
package-review@lists.fedoraproject.org