[Bug 477338] Review Request: php-pecl-imagick - Provides a wrapper to the ImageMagick library

[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=477338





--- Comment #6 from Pavel Alexeev <pahan at hubbitus.spb.su>  2009-01-11 08:31:22 EDT ---
Firstly - thank you for review.

(In reply to comment #4)
> REVIEW:

> SHOULD:
> - TODO is provided empty by upstream, so i think it's ok (could be not empty in
> another version)
> 
> - INSTALL is also empty but generally provides information about "building from
> source" which is not usefull with RPM. You should probably remove it (some
> package keep it, some remove it)
As mentioned initialy in first post I also thik what it is not errors and I
want to stay it here because it comes from upstream.

> - add version in BR (ok for all fedora but EPEL 4 don't have requirement)
> BuildRequires: php-devel >= 5.1.3, php-pear, ImageMagick-devel >= 6.2.4
Ok.
But from what you get minimum requirement version of ImageMagick? I do not
remember what versions was used, but as you can see by changelog, I maintain
(for himself repository off course) this package notably long time, nad do not
remember any problems with IM...

> - setup the -n option is needless when -c used
I wasn't known that. Fixed.

> - add conditional (recommended in PHP Guidelines)
>   %post => %if 0%{?pecl_install:1}
>   %postun => %if 0%{?pecl_uninstall:1}
Done.
> Without, you package couldn't be imported in EPEL-5 (macro not defined in old
> php-pear, but even "pecl install" don't work => no extension registration in
> this case)
So, EPEL-5 do not require such registration at all??

> 
> MUST:
> - add the missing virtual provides (from PHP guidelines)
> Provides:     php-pecl(%peclName)
Ok, added:
Provides:     php-pecl(%peclName) = %{version}

> 
> Do you want to maintain thi package in EPEL ?
I'm not use this distributions itself. But I can maintain it for EPEL5 on
CentOS. It is acceptable? I compleatly do not want maintain it for EPEL4 (but I
can import for that branch without testing with hope to the best). May be you
want co-mantain it in EPEL4 and/or EPEL5?


http://hubbitus.net.ru/rpm/Fedora9/php-pecl-imagick/php-pecl-imagick-2.2.1-3.fc9.src.rpm

-- 
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.