Bug 785441 (Horde_Nls)

Summary: Review Request: php-horde-Horde-Nls - Native Language Support (NLS)
Product: [Fedora] Fedora Reporter: Nick Bebout <nb>
Component: Package ReviewAssignee: Remi Collet <fedora>
Status: CLOSED CURRENTRELEASE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: fedora, notting, package-review
Target Milestone: ---Flags: fedora: fedora-review+
gwync: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2012-03-21 23:49:11 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Bug Depends On: 785424, 785432, 785439    
Bug Blocks: 785442, 785463    

Description Nick Bebout 2012-01-28 23:57:27 UTC
Spec URL: http://nb.fedorapeople.org/horde-reviews/php-horde-Horde-Nls.spec
SRPM URL: http://nb.fedorapeople.org/horde-reviews/php-horde-Horde-Nls-1.1.3-1.fc16.src.rpm
Description: Common methods for handling language data, timezones, and hostname->country lookups.

Comment 1 Remi Collet 2012-01-30 18:48:42 UTC
--- php-horde-Horde-Nls.spec.old	2012-01-30 19:38:29.000000000 +0100
+++ php-horde-Horde-Nls.spec	2012-01-30 19:46:15.000000000 +0100
@@ -12,14 +12,15 @@
 Source0:        http://pear.horde.org/get/%{pear_name}-%{version}.tgz
 
 BuildArch:      noarch
-BuildRequires:  php-pear >= 1:1.4.9-1.2
+BuildRequires:  php-pear(PEAR) >= 1.7.0
+BuildRequires:  php-channel(pear.horde.org)
+
 Requires(post): %{__pear}
 Requires(postun): %{__pear}
-Requires:       php-pear(pear.horde.org/Horde_Translation) <= 2.0.0, php-pear(pear.horde.org/Horde_Util) <= 2.0.0, php-pear(PEAR) >= 1.7.0
-Conflicts:      php-pear(pear.horde.org/Horde_Translation) = 2.0.0, php-pear(pear.horde.org/Horde_Util) = 2.0.0
-Provides:       php-pear(pear.horde.org/Horde_Nls) = %{version}
-BuildRequires:  php-channel(pear.horde.org)
-Requires:       php-channel(pear.horde.org)
+Requires:       php-pear(pear.horde.org/Horde_Util) >= 1.0.0
+Requires:       php-pear(pear.horde.org/Horde_Util) <  2.0.0
+Requires:       php-pear(PEAR) >= 1.7.0
+Provides:       php-pear(pear.horde.org/%{Horde_Nls}) = %{version}
 
 

I propose to remove Horde_Translation, already required by dependency of Horde_Util (you could keep it, if you think "versionning" have some value)

Requiring the channel not needed (I miss to drop this on other packages, except Translation which don't requires other horde package)

+ must handle locales

NB optional dep on Net_DNS2 (not yet available in fedora) and geoip (available). As rpm doesn't handle optional dep, I always think of adding this are normal dep (when not pull to much things => but this is your choice

Comment 2 Nick Bebout 2012-02-01 03:04:57 UTC
I'm confused.  According to http://fedoraproject.org/wiki/Packaging:PHP#PEAR_Packages_from_a_non_standard_channel.2Frepository it seems to indicate that I must require the channel?

Comment 3 Remi Collet 2012-02-01 06:13:37 UTC
Yes, You must requires it for a single package, but as far as you requires another package in this channel, you already "implicitly" requires it.

After a look to some packages, as all requires
php-pear(PEAR) >= 1.7.0
php-common >= 5.2.0

You could even add this 2 requirement in the channel, to avoid adding it (BR and R) in each package. This will make the dependency stack really simpler.

About "locales", any feedback from upstream ?

After discussion on IRC, "all files must be generated from sources", so the .mo should be created during the rpmbuild, and .po/.pot doesn't need to be packaged (except if you have a good explanation about why they are required)

This will make the spec a little more complex, but will ensure that the .mo will be accurate, with the .po, and with the gettext version.

Comment 4 Nick Bebout 2012-02-15 22:05:41 UTC
I believe all outstanding issues are fixed now.

Comment 5 Remi Collet 2012-02-19 07:47:13 UTC
> NB optional dep on Net_DNS2 (not yet available in fedora) and geoip
> (available). As rpm doesn't handle optional dep, I always think of adding
> this as normal dep (when not pull to much things) => but this is your
> choice

This one is also ok now.

APPROVED

Comment 6 Nick Bebout 2012-02-19 14:42:09 UTC
New Package SCM Request
=======================
Package Name: php-horde-Horde-Nls
Short Description: Common methods for handling language data, timezones, and
hostname->country lookups
Owners: nb
Branches: f16 f17 el6
InitialCC:

Comment 7 Gwyn Ciesla 2012-02-19 20:42:25 UTC
Git done (by process-git-requests).