Bug 486044 - Review Request: php-pear-Config -Configuration file manipulation for PHP
Review Request: php-pear-Config -Configuration file manipulation for PHP
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Remi Collet
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2009-02-17 19:47 EST by Bernard Johnson
Modified: 2009-03-09 19:07 EDT (History)
3 users (show)

See Also:
Fixed In Version: 1.10.11-3.fc10
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-03-09 19:07:48 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
fedora: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Bernard Johnson 2009-02-17 19:47:43 EST
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 02:29:54 EST
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-25 19:23:38 EST
(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@symetrix.com> - 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 11:37:38 EST
> 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 12:17:02 EST
(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-02 19:14:52 EST
cvs done.
Comment 6 Fedora Update System 2009-03-07 12:31:11 EST
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 19:07:42 EDT
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.