Bug 853463

Summary: Review Request: php-redis - Extension for communicating with the Redis key-value store
Product: [Fedora] Fedora Reporter: Remi Collet <fedora>
Component: Package ReviewAssignee: Shawn Iwinski <shawn>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: notting, package-review, shawn
Target Milestone: ---Flags: shawn: 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-09-25 16:36:07 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Attachments:
Description Flags
Generated by fedora-review 0.2.2 (9f8c0e5) last change: 2012-08-09 none

Description Remi Collet 2012-08-31 15:57:07 UTC
Spec URL: https://raw.github.com/remicollet/remirepo/master/php/php-redis/php-redis.spec
SRPM URL: http://rpms.famillecollet.com/SRPMS/php-redis-2.2.2-2.git5df5153.remi.src.rpm
Description: 
The phpredis extension provides an API for communicating
with the Redis key-value store.

Fedora Account System Username: remi


I use the latest master from github (instead of taggued 2.2.2) because I work with upstream to provides a warning free build and a usable test suite.

Koji scratch Build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=4441684

Target : all branch (including EPEL-5, even if I have not yet test the build)

Comment 1 Remi Collet 2012-09-01 08:11:04 UTC
Minor change: run test suite only is redis > 2.4 available

https://github.com/remicollet/remirepo/commit/ce9341e4e4f5610291fa2a25025ea78d9db447b3

SRPM: http://rpms.famillecollet.com/SRPMS/php-redis-2.2.2-3.git5df5153.remi.src.rpm

EPEL-5 not targeted (redis requires igbinary which requires php 5.2) but I want to keep old packaging stuff (buildroot, clean, ...) to make backports possible.

Comment 2 Remi Collet 2012-09-01 10:19:24 UTC
EPEL-6 koji scratch build
http://koji.fedoraproject.org/koji/taskinfo?taskID=4443754

Comment 3 Remi Collet 2012-09-02 18:28:47 UTC
Latest changes from upstream (mainly useful for packaging)
- clean right on sources (avoid chmod in spec)
- remove bundled igbinary (avoid hack in spec)

https://github.com/remicollet/remirepo/commit/e38017592917389bb37102a80d988aaf0f7dbaf3

SRPM: http://rpms.famillecollet.com/SRPMS/php-redis-2.2.2-4.git6f7087f.remi.src.rpm

Comment 4 Shawn Iwinski 2012-09-10 23:13:20 UTC
A couple questions before approval:
1) Is there a reason you used "%{_sysconfdir}/php.d" instead of "%{php_inidir}"?
2) Is there a reason you left arrays.markdown out of the documentation (it looks like it may be useful to end users)

Comment 5 Remi Collet 2012-09-11 04:28:15 UTC
(In reply to comment #4)
> A couple questions before approval:
> 1) Is there a reason you used "%{_sysconfdir}/php.d" instead of
> "%{php_inidir}"?

%{php_inidir} is only define in Fedora (php 5.4), not in RHEL (php 5.3)

> 2) Is there a reason you left arrays.markdown out of the documentation (it
> looks like it may be useful to end users)

good catch (as for serialize.list)


https://github.com/remicollet/remirepo/commit/9010319213503293e6ff213d286baa4a6d0ddf27

New SRPM: http://rpms.famillecollet.com/SRPMS/php-redis-2.2.2-5.git6f7087f.remi.src.rpm

Comment 6 Shawn Iwinski 2012-09-12 21:47:47 UTC
(In reply to comment #5)
> (In reply to comment #4)
> > A couple questions before approval:
> > 1) Is there a reason you used "%{_sysconfdir}/php.d" instead of
> > "%{php_inidir}"?
> 
> %{php_inidir} is only define in Fedora (php 5.4), not in RHEL (php 5.3)

Unless I'm missing something, I don't see %{php_ztsinidir} or %{php_ztsextdir} being defined in RHEL (php-5.3.3-14.el6_3) either.  Please replace in spec.

Comment 7 Remi Collet 2012-09-13 04:21:03 UTC
(In reply to comment #6)
> Unless I'm missing something, I don't see %{php_ztsinidir} or
> %{php_ztsextdir} being defined in RHEL (php-5.3.3-14.el6_3) either.  Please
> replace in spec.

All the ZTS stuff is protected by the "with_zts" condition.

In RHEL and Fedora <= 16, __ztsphp is not defined, so with_zts is false.
In Fedora >= 17 __ztsphp, php_ztsinidir, php_ztsextdir are defined.

See EPEL-6 koji scratch build in comment #2.

Comment 8 Shawn Iwinski 2012-09-13 11:27:59 UTC
(In reply to comment #7)
> (In reply to comment #6)
> > Unless I'm missing something, I don't see %{php_ztsinidir} or
> > %{php_ztsextdir} being defined in RHEL (php-5.3.3-14.el6_3) either.  Please
> > replace in spec.
> 
> All the ZTS stuff is protected by the "with_zts" condition.
> 
> In RHEL and Fedora <= 16, __ztsphp is not defined, so with_zts is false.
> In Fedora >= 17 __ztsphp, php_ztsinidir, php_ztsextdir are defined.
> 
> See EPEL-6 koji scratch build in comment #2.

Sorry I missed that :(  I blame it on being sick for the past few days.  I will post approved review in a couple hours (I have a few other things to finish up).

Comment 9 Shawn Iwinski 2012-09-13 20:57:50 UTC
Created attachment 612642 [details]
Generated by fedora-review 0.2.2 (9f8c0e5) last change: 2012-08-09

Comment 10 Shawn Iwinski 2012-09-13 20:58:18 UTC
No blockers.

==== APPROVED ====

Comment 11 Remi Collet 2012-09-14 04:28:11 UTC
Thanks for the review.


New Package SCM Request
=======================
Package Name: php-redis
Short Description: Extension for communicating with the Redis key-value store
Owners: remi
Branches: f17 f18 el6
InitialCC:

Comment 12 Gwyn Ciesla 2012-09-14 11:01:34 UTC
Git done (by process-git-requests).

Comment 13 Fedora Update System 2012-09-14 12:59:17 UTC
php-redis-2.2.2-5.git6f7087f.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/php-redis-2.2.2-5.git6f7087f.fc17

Comment 14 Fedora Update System 2012-09-14 13:01:26 UTC
php-redis-2.2.2-5.git6f7087f.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/php-redis-2.2.2-5.git6f7087f.el6

Comment 15 Fedora Update System 2012-09-14 13:11:45 UTC
php-redis-2.2.2-5.git6f7087f.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/php-redis-2.2.2-5.git6f7087f.fc18

Comment 16 Fedora Update System 2012-09-16 17:09:53 UTC
Package php-redis-2.2.2-5.git6f7087f.fc18:
* should fix your issue,
* was pushed to the Fedora 18 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing php-redis-2.2.2-5.git6f7087f.fc18'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2012-13941/php-redis-2.2.2-5.git6f7087f.fc18
then log in and leave karma (feedback).

Comment 17 Fedora Update System 2012-09-25 16:36:07 UTC
php-redis-2.2.2-5.git6f7087f.fc18 has been pushed to the Fedora 18 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 18 Fedora Update System 2012-09-26 08:52:41 UTC
php-redis-2.2.2-5.git6f7087f.fc17 has been pushed to the Fedora 17 stable repository.  If problems still persist, please make note of it in this bug report.