Bug 486009 - Review Request: php-pear-Crypt-Blowfish - Quick two-way blowfish encryption
Summary: Review Request: php-pear-Crypt-Blowfish - Quick two-way blowfish encryption
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
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:
Blocks: 486010
TreeView+ depends on / blocked
 
Reported: 2009-02-17 21:12 UTC by Xavier Bachelot
Modified: 2009-04-17 16:30 UTC (History)
4 users (show)

Fixed In Version: 1.1.0-0.3.rc2.fc10
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-02-25 16:22:38 UTC
Type: ---
Embargoed:
fedora: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Xavier Bachelot 2009-02-17 21:12:10 UTC
Spec URL: http://www.bachelot.org/fedora/SPECS/php-pear-Crypt_Blowfish.spec
SRPM URL: http://www.bachelot.org/fedora/SRPMS/php-pear-Crypt_Blowfish-1.1.0-0.1.rc2.fc10.src.rpm
Description: 
This package allows you to perform two-way blowfish encryption on the fly using only PHP. This package does not require the MCrypt PHP extension to work, although it can make use of it if available.

Comment 1 Remi Collet 2009-02-22 07:17:20 UTC
First notes :
- It requires PEAR >= 1.7.2 (according to upstream and to xml) : this will block this package for EPEL.

- rpmlint warning :
W: summary-not-capitalized quick two-way blowfish encryption

- %file must be fixed
%files
%defattr(-,root,root,-)
%{pear_xmldir}/%{pear_name}.xml
%{pear_testdir}/%{pear_name}
%{pear_phpdir}/Crypt

- I would prefer using %{name}.xml rather than %{pear_name}.xml (see recently approved PHP Guidelines, this is usefull to avoid conflict between package from various channel)

- a comment about running the tests (which must be done as root after install) will be usefull :
=> pear run-tests -p Crypt_Blowfish

Comment 2 Christopher Stone 2009-02-22 17:47:35 UTC
Shouldn't this be renamed to php-pear-Crypt-Blowfish to meet Fedora package naming guidelines?

Comment 3 Remi Collet 2009-02-22 18:00:17 UTC
Yes Christopher, you're right, I've miss this error (but it's only first notes ;) not the formal review).

Comment 4 Xavier Bachelot 2009-02-22 23:08:42 UTC
Renamed and updated to follow suggestions :
Spec URL: http://www.bachelot.org/fedora/SPECS/php-pear-Crypt-Blowfish.spec
SRPM URL: http://www.bachelot.org/fedora/SRPMS/php-pear-Crypt-Blowfish-1.1.0-0.2.rc2.fc10.src.rpm

Comment 5 Remi Collet 2009-02-23 16:29:32 UTC
You should remove (probably a paste error, already defined above)
----------------------------
BuildRequires: php-pear(PEAR)
Requires: php-pear(PEAR)
Requires(post): %{__pear}
Requires(postun): %{__pear}
Provides:     php-pear(foo) = %{version}
----------------------------

Comment 6 Remi Collet 2009-02-23 16:54:21 UTC
REVIEW:

+ rpmlint is ok
php-pear-Crypt-Blowfish.src: I: checking
php-pear-Crypt-Blowfish.noarch: I: checking
php-pear-Crypt-Blowfish.noarch: W: no-documentation
2 packages and 1 specfiles checked; 0 errors, 1 warnings.
+ package name
+ spec file name 
+ package meet the PHP Guidelines 
+ License ok : BSD
+ License is upstream 
+ spec in english and legible
+ no license file in sources
+ sources match the upstream sources
09f0e38a4d524ba4102db5d11b07ffe9  Crypt_Blowfish-1.1.0RC2.tgz
+ Source URL ok
+ build / run on F10.x86_64
+ BuildRequires (php-pear >= 1:1.4.9-1.2) ok
+ no locale
+ no .so
+ own all directories that it creates
+ no duplicate file
+ %defattr ok
+ %clean section
+ use macros consistently
+ contain code
+ no documentation
+ no devel
+ no pkgconfig
+ no sub-package
+ no GUI
+ don't own files or directories already owned by other packages
+ %install start with rm -rf 
+ valid UTF-8
+ build in mock (fedora-rawhide-x86_64)
+ test suite ok (2 PASSED TESTS)
+ scriptlets ok
- Final Requires ok, but see previous comments
- Final Provides ko, see previous comments
php-pear(foo) = 1.1.0

Just fix the previous comment and I will approve this package.

Comment 7 Xavier Bachelot 2009-02-23 19:27:32 UTC
Thanks for the review, Remi. This is indeed a cut and paste error...

Fixed version :
Spec URL: http://www.bachelot.org/fedora/SPECS/php-pear-Crypt-Blowfish.spec
SRPM URL:
http://www.bachelot.org/fedora/SRPMS/php-pear-Crypt-Blowfish-1.1.0-0.3.rc2.fc10.src.rpm

Comment 8 Remi Collet 2009-02-24 05:50:14 UTC
+ Requires
php-pear(PEAR)

+ Provides
php-pear(Crypt_Blowfish) = 1.1.0
php-pear-Crypt-Blowfish = 1.1.0-0.3.rc2.fc8

APPROVED

Comment 9 Xavier Bachelot 2009-02-24 08:43:42 UTC
New Package CVS Request
=======================
Package Name: php-pear-Crypt-Blowfish
Short Description: Quick two-way blowfish encryption
Owners: xavierb
Branches: F-9 F-10
InitialCC:

Comment 10 Kevin Fenzi 2009-02-24 20:42:58 UTC
cvs done.

Comment 11 Fedora Update System 2009-02-25 00:10:55 UTC
php-pear-Crypt-Blowfish-1.1.0-0.3.rc2.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/php-pear-Crypt-Blowfish-1.1.0-0.3.rc2.fc10

Comment 12 Xavier Bachelot 2009-02-25 00:13:46 UTC
Imported and built for F-10 and devel.
Doesn't build on F-9 because php-pear version is too old (available 1.7.1, required 1.7.2)

Comment 13 Fedora Update System 2009-02-25 16:22:33 UTC
php-pear-Crypt-Blowfish-1.1.0-0.3.rc2.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 14 Xavier Bachelot 2009-04-15 10:20:26 UTC
Remi, the earlier 1.1.0 rc1 or 1.0.1 versions don't have a require on pear >= 1.7.2, this would allow to build for F9 and EL5. Would you object if I build either version ?

Comment 15 Xavier Bachelot 2009-04-16 09:21:48 UTC
Package Change Request
======================
Package Name: php-pear-Crypt-Blowfish
New Branches: EL-5
Owners: xavierb

Comment 16 Kevin Fenzi 2009-04-17 16:30:45 UTC
cvs done.


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