Bug 486044 - Review Request: php-pear-Config -Configuration file manipulation for PHP
Summary: Review Request: php-pear-Config -Configuration file manipulation for PHP
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:
TreeView+ depends on / blocked
 
Reported: 2009-02-18 00:47 UTC by Bernard Johnson
Modified: 2009-03-09 23:07 UTC (History)
3 users (show)

Fixed In Version: 1.10.11-3.fc10
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-03-09 23:07:48 UTC
Type: ---
Embargoed:
fedora: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Bernard Johnson 2009-02-18 00:47:43 UTC
Spec URL: http://fedorapeople.org/~bjohnson/php-pear-Config.spec
SRPM URL: http://fedorapeople.org/~bjohnson/php-pear-Config-1.10.11-1.fc10.src.rpm
Description: 
The PHP Config package provides methods for configuration manipulation.
* Creates configurations from scratch
* Parses and outputs different formats (XML, PHP, INI, Apache...)
* Edits existing configurations
* Converts configurations to other formats
* Allows manipulation of sections, comments, directives...
* Parses configurations into a tree structure

Comment 1 Remi Collet 2009-02-22 07:29:54 UTC
First Notes :

- As optional deps are available it should be usefull to add it
Requires: php-pear(XML_Parser) php-pear(XML_Util)

- I prefer the use of %{name}.xml rather than %{pear_name}.xml (see recent change to PHP Guidelines, this will avoid conflict with package from other channel)

- rpmlint is silent

- A comment about running test-suite will be usefull
# pear run-tests -p Config
Running 16 tests
...
FAIL [ 3/16] test for bug 3051[/usr/share/pear/test/Config/test/bug3051.phpt]
...
FAIL [16/16] regression test for bug #10185[/usr/share/pear/test/Config/test/bug10185.phpt]
wrote log to "/root/run-tests.log"
TOTAL TIME: 00:01
14 PASSED TESTS
0 SKIPPED TESTS
2 FAILED TESTS:
/usr/share/pear/test/Config/test/bug3051.phpt
/usr/share/pear/test/Config/test/bug10185.phpt

Have you encounter and investigate this issue ? (at least, reported upstream)

- %file must be fixed, should be
%defattr(-,root,root,-)
%doc %{pear_name}-%{version}/docdir/%{pear_name}/*
%{pear_xmldir}/%{pear_name}.xml
%{pear_testdir}/%{pear_name}
%{pear_phpdir}/Config*

Comment 2 Bernard Johnson 2009-02-26 00:23:38 UTC
(In reply to comment #1)
> First Notes :
> 
> - As optional deps are available it should be usefull to add it
> Requires: php-pear(XML_Parser) php-pear(XML_Util)

added

> - I prefer the use of %{name}.xml rather than %{pear_name}.xml (see recent
> change to PHP Guidelines, this will avoid conflict with package from other
> channel)

I did not find anything in the PHP guidelines, but I think I understand your intention.  Please check and see if my updates to the package achieve this.

> - A comment about running test-suite will be usefull
> 14 PASSED TESTS
> 0 SKIPPED TESTS
> 2 FAILED TESTS:
> Have you encounter and investigate this issue ? (at least, reported upstream)

Two of the tests are badly written and fail.  There is no problem with the functionality of the package.

> 
> - %file must be fixed, should be
> %{pear_phpdir}/Config*

fixed

Spec URL: http://fedorapeople.org/~bjohnson/php-pear-Config.spec
SRPM URL:
http://fedorapeople.org/~bjohnson/php-pear-Config-1.10.11-2.fc10.src.rpm

* Wed Feb 25 2009 Bernard Johnson <bjohnson> - 1.10.11-2
- add dependencies for php-pear(XML_Parser) and php-pear(XML_Util)
- change from %%{pear_name}.xml to %%{name}.xml to avoid channel conflicts
- changes %%files section from %%{pear_phpdir}/* to %%{pear_phpdir}/Config*
- note regarding test suite failures added

Comment 3 Remi Collet 2009-02-26 16:37:38 UTC
> Two of the tests are badly written and fail.  There is no problem with the
> functionality of the package.

This should be reported upstream

REVIEW:

+ rpmlint is ok
php-pear-Config.src: I: checking
php-pear-Config.noarch: I: checking
2 packages and 1 specfiles checked; 0 errors, 0 warnings.
+ package name
+ spec file name 
+ package meet the PHP Guidelines (new update)
+ License ok : BSD
+ License is upstream 
+ spec in english and legible
+ no license file in sources is provided
+ sources match the upstream sources
ec85ece7ddd28a0a139c0699481c0116  Config-1.10.11.tgz
+ Source URL ok
+ build  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
+ small 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 : see previous comment
+ scriptlets ok
+ Final Requires ok
/bin/sh  
/usr/bin/pear  
php-pear(PEAR)  
php-pear(XML_Parser)  
php-pear(XML_Util)  
+ Final Provides ok
php-pear(Config) = 1.10.11
php-pear-Config = 1.10.11-2.fc8


php-pear(PEAR) should be removed from Requires as already required by php-pear(XML_Parser) and php-pear(XML_Util)

APPROVED

Comment 4 Bernard Johnson 2009-03-01 17:17:02 UTC
(In reply to comment #3)
> > Two of the tests are badly written and fail.  There is no problem with the
> > functionality of the package.
> 
> This should be reported upstream

Reported


> php-pear(PEAR) should be removed from Requires as already required by
> php-pear(XML_Parser) and php-pear(XML_Util)

I will remove this after the import.  Thanks for the review!



New Package CVS Request
=======================
Package Name: php-pear-Config
Short Description: Configuration file manipulation for PHP
Owners: bjohnson
Branches: F-10
InitialCC:

Comment 5 Kevin Fenzi 2009-03-03 00:14:52 UTC
cvs done.

Comment 6 Fedora Update System 2009-03-07 17:31:11 UTC
php-pear-Config-1.10.11-3.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/php-pear-Config-1.10.11-3.fc10

Comment 7 Fedora Update System 2009-03-09 23:07:42 UTC
php-pear-Config-1.10.11-3.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.


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