Bug 218230 - (pear-Image-Canvas) Review Request: php-pear-Image-Canvas - Common interface to image drawing
Review Request: php-pear-Image-Canvas - Common interface to image drawing
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Remi Collet
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-12-03 17:32 EST by Christopher Stone
Modified: 2007-11-30 17:11 EST (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-01-05 10:04:23 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
wtogami: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Christopher Stone 2006-12-03 17:32:51 EST
Spec URL: http://tkmame.retrogames.com/fedora-extras/php-pear-Image-Canvas.spec
SRPM URL: http://tkmame.retrogames.com/fedora-extras/php-pear-Image-Canvas-0.3.0-1.src.rpm

Description:
A package providing a common interface to image drawing, making
image source code independent on the library used.
Comment 1 Remi Collet 2006-12-07 13:38:53 EST
Image/Canvas/PDF.php requires php-pecl-pdflib.

I provide php-pecl-pdflib and pdflib-lite on my repo (for a long time) but i
dont have submit them to review (yet) because i'm not certain that License is OK.

With pdflib present, test doesn't work because of Font used ("Courier New"
defined as default one in Image/Canvas.php not available).

A temporary solution could be to disable Image/PDF.php

I will investigate on this...
Comment 2 Christopher Stone 2006-12-07 16:50:15 EST
I have updated the spec to remove this file, however I think it would be better
to add a php-pecl-pdflib package to Extras.  I think the license should be okay.
 If you want to put it up for review I can review it for you.  Once it's in I
can also patch this package to not use Courier New font in the tests.

Spec URL: http://tkmame.retrogames.com/fedora-extras/php-pear-Image-Canvas.spec
SRPM URL:
http://tkmame.retrogames.com/fedora-extras/php-pear-Image-Canvas-0.3.0-2.src.rpm

%changelog
* Thu Dec 07 2006 Christopher Stone <chris.stone@gmail.com> 0.3.0-2
- Remove PDF.php until php-pecl-pdflib is added to Extras
Comment 3 Remi Collet 2006-12-09 01:55:27 EST
Tests works. Example doesn't because it's use MS Windows Fonts.

REVIEW
* source files match upstream:
41dd36fb05436159fb6fccca02cb7aaa  /tmp/Image_Canvas-0.3.0.tgz
41dd36fb05436159fb6fccca02cb7aaa  ../SOURCES/Image_Canvas-0.3.0.tgz
* package meets naming and packaging guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* dist tag is present.
* build root is correct.
* license field matches the actual license.
* license is open source-compatible (LGPL).
* latest version is being packaged (0.3.0)
* BuildRequires are proper.
* %clean is present.
* package builds in mock (development).
* package installs properly
* rpmlint is silent.
* final provides are sane:
php-pear(Image_Canvas) = 0.3.0
php-pear-Image-Canvas = 0.3.0-2.fc7
* final Requires are sane:
php-pear(Image_Color) -> php-gd
* %check is not present; 
* owns the directories it creates
=> own directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* scriptlets are OK (pear install)
* code, not content.
* documentation is small, so no -docs subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.

MUST :
- not own /usr/share/pear/Image which is owned by php-pear-Image-Color (required
, remplace with 
%file /usr/share/pear/Image/Canvas*

SHOULD : 
- also remove /usr/share/pear/test/Image_Canvas/tests/pdf.php
- add a information (in description or a README.fedora) about "PDF not available"
Comment 4 Remi Collet 2006-12-09 01:59:09 EST
> I think it would be better to add a php-pecl-pdflib package to Extras. 
> I think the license should be okay.

php-pecl-pdflib is PHP License
It's the "PDFlib Lite License" which is a special License
Comment 5 Christopher Stone 2006-12-09 12:46:53 EST
Ah okay, I didn't realize pdflib-lite was needed for pecl-pdflib.  I've
discussed the license for pdflib-lite briefly on IRC and initial discussion
seemed to indicate it was okay.  I'm going to wait a few days to see if there
are any objections, and if we can get it in Extras then I'll modify this package
to use php-pecl-pdflib and readd the PDF files.
Comment 6 Christopher Stone 2006-12-14 11:47:57 EST
Okay, looks like the pdflib stuff can't go into Extras, I'll go ahead and start
working on the items you mentioned in comment #3.
Comment 7 Christopher Stone 2006-12-14 11:51:38 EST
I think perhaps it would be better to leave the PDF files in the RPM.  Even
though they require a package which cannot be included in Extras, I think we
should keep the files there anyway incase someone wants to install the packages
from another repository such as Remi's repo.  However I agree that adding a
README.Fedora file explaining this should be required.  Please let me know if
you think this is not a good idea.  Thanks.
Comment 8 Remi Collet 2006-12-14 12:31:22 EST
> I think we should keep the files there anyway
Ok for me

> However I agree that adding a README.Fedora file explaining this should be
required
Of course, i also think it is a MUST.

Comment 9 Christopher Stone 2007-01-03 13:28:49 EST
Spec URL: http://tkmame.retrogames.com/fedora-extras/php-pear-Image-Canvas.spec
SRPM URL:
http://tkmame.retrogames.com/fedora-extras/php-pear-Image-Canvas-0.3.0-3.src.rpm

%changelog
* Wed Jan 03 2007 Christopher Stone <chris.stone@gmail.com> 0.3.0-3
- No longer remove PDF.php file
- Add README.Fedora file exlaining requirements for PDF.php
Comment 10 Christopher Stone 2007-01-03 13:34:40 EST
oops this bit got cut off from changelog paste above:
- No longer own Image directory
Comment 11 Remi Collet 2007-01-05 02:33:25 EST
* owns the directories it creates
* don't own directories it shouldn't.

APPROVED
Comment 12 Christopher Stone 2007-01-05 10:04:23 EST
- Imported into CVS
- Added entry to owners.list
- Build succeeded for devel
- Requested FC-5/6 CVS sync

Thanks for the review!
Comment 13 Christopher Stone 2007-04-29 14:37:47 EDT
Package Change Request
======================
Package Name: php-pear-Image-Canvas
New Branches: EL-5

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