Bug 785447 (Horde_Auth) - Review Request: php-horde-Horde-Auth - Horde Authentication API
Summary: Review Request: php-horde-Horde-Auth - Horde Authentication API
Keywords:
Status: CLOSED ERRATA
Alias: Horde_Auth
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Remi Collet
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: horde-channel Horde_Exception Horde_Util
Blocks: Horde_Core Horde_Vfs Horde_SyncMl Horde_Kolab_Server
TreeView+ depends on / blocked
 
Reported: 2012-01-29 00:57 UTC by Nick Bebout
Modified: 2013-04-05 23:04 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2013-04-04 16:38:21 UTC
Type: ---
fedora: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)
phpci.log (22.06 KB, text/plain)
2013-03-27 09:10 UTC, Remi Collet
no flags Details
review.txt (6.89 KB, text/plain)
2013-03-27 09:12 UTC, Remi Collet
no flags Details

Description Nick Bebout 2012-01-29 00:57:43 UTC
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.

Comment 1 Nick Bebout 2012-06-14 21:53:46 UTC
I believe this one is ready to review now.

Comment 3 Shawn Iwinski 2012-07-04 23:25:57 UTC
I will review this package

Comment 4 Nick Bebout 2012-07-19 16:46:24 UTC
I believe all of the normal blockers for the php-horde-Horde-* packages are fixed with this package.

Comment 5 Shawn Iwinski 2012-07-21 19:09:12 UTC
[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"

Comment 6 Shawn Iwinski 2012-07-21 21:20:05 UTC
(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

Comment 9 Remi Collet 2013-01-17 06:44:12 UTC
quick notes:
- missing %check
- missing %{php_metadir}
- must own %{pear_datadir}/Horde_Auth/migration
- should use %{pear_name} (and perhaps %{channel_name})

Comment 10 Remi Collet 2013-01-17 06:46:27 UTC
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.

Comment 12 Remi Collet 2013-02-10 08:41:39 UTC
Shawn, do you plan to review this one ? (the next according to my build order)

Comment 13 Shawn Iwinski 2013-02-10 17:17:45 UTC
(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.

Comment 14 Shawn Iwinski 2013-02-10 17:24:20 UTC
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.

Comment 16 Remi Collet 2013-03-27 09:10:17 UTC
Created attachment 716958 [details]
phpci.log

phpci 2.14.0

Comment 17 Remi Collet 2013-03-27 09:12:13 UTC
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

Comment 18 Remi Collet 2013-03-27 09:13:04 UTC
[!]: 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)

Comment 21 Remi Collet 2013-04-02 10:55:29 UTC
-%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 ===

Comment 22 Nick Bebout 2013-04-03 01:01:11 UTC
New Package SCM Request
=======================
Package Name: php-horde-Horde-Auth
Short Description: Horde Authentication API
Owners: nb remi
Branches: el6 f18 f19
InitialCC:

Comment 23 Gwyn Ciesla 2013-04-03 11:51:05 UTC
Git done (by process-git-requests).

Comment 24 Fedora Update System 2013-04-03 21:11:44 UTC
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

Comment 25 Fedora Update System 2013-04-03 21:34:49 UTC
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

Comment 26 Fedora Update System 2013-04-04 16:38:26 UTC
php-horde-Horde-Auth-2.0.4-2.el6 has been pushed to the Fedora EPEL 6 stable repository.

Comment 27 Fedora Update System 2013-04-05 23:04:21 UTC
php-horde-Horde-Auth-2.0.4-2.fc18 has been pushed to the Fedora 18 stable repository.


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