[Bug 560071] Review Request: php-pecl-augeas - PHP bindings to the Augeas API

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=560071

Remi Collet <fedora at famillecollet.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |fedora at famillecollet.com

--- Comment #5 from Remi Collet <fedora at famillecollet.com> 2010-02-06 12:35:59 EST ---
Just some quick notes :

- missing CREDITS files in %doc

- rpmlint output a error
E: explicit-lib-dependency augeas-libs

You don't need to "requires: augeas-libs" as the binary package auto-requires
"libaugeas.so.0" (provided by augeas-libs)

- %define (line 2) must be replaced by %global

- must not requires "php" (which brings a lot other unneeded dependencies,
mainly httpd), but "php-common"

- conditional requires not needed (as this requires php 5.2, which always
provides php(zend-abi))

Instead of:
Requires: php >= 5.2.0, augeas-libs
%if 0%{?php_zend_api}
Requires: php(zend-abi) = %{php_zend_api}
Requires: php(api) = %{php_core_api}
%else
Requires: php-api = %{php_apiver}
%endif

Could simply use:
Requires:     php-common >= 5.2.0
Requires:     php(zend-abi) = %{php_zend_api}
Requires:     php(api) = %{php_core_api}


- you create a macro %{pecl_name} macro, but you don't use it where you could

- your macro usage is not consistent. As you use sometime "%{__install}" and
sometime "install"

- no test provided, but you can add a simple load test

%check
%{_bindir}/php \
    -n -q -d extension_dir=modules \
    -d extension=%{pecl_name}.so \
    --modules | grep %{pecl_name}


What are your other reviews pending ? 
(if under my skills, I could enter in the process to sponsor you)

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.