Bug 438892 - Review Request: pyPdf - PDF toolkit
Summary: Review Request: pyPdf - PDF toolkit
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Paul Howarth
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2008-03-25 20:15 UTC by Felix Schwarz
Modified: 2008-05-13 17:07 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2008-04-13 17:50:36 UTC
paul: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Felix Schwarz 2008-03-25 20:15:54 UTC
Spec URL: http://www.felix-schwarz.name/files/misc/2008/pypdf/pypdf.spec
SRPM URL: http://www.felix-schwarz.name/files/misc/2008/pypdf/pyPdf-1.10-1.fc8.src.rpm
Description: 
A Pure-Python library built as a PDF toolkit. It is capable of:
 * extracting document information (title, author, ...),
 * splitting documents page by page,
 * merging documents page by page,
 * cropping pages,
 * merging multiple pages into a single page,
 * encrypting and decrypting PDF files.

As this is my first RPM which I submit to Fedora, I need a sponsor.

Comment 1 Nicolas A. Corrarello 2008-03-25 20:28:53 UTC
Rpmlint says:
pyPdf.src: E: invalid-spec-name pypdf.spec
pyPdf.src: W: invalid-license modified BSD



Comment 2 Felix Schwarz 2008-03-25 22:10:18 UTC
Sorry, I ran rpmlint only on the spec file.

Updated version:
http://www.felix-schwarz.name/files/misc/2008/pypdf/pyPdf.spec
http://www.felix-schwarz.name/files/misc/2008/pypdf/pyPdf-1.10-2.fc8.src.rpm


Comment 3 Felix Schwarz 2008-03-27 21:13:21 UTC
New version:
- removed unnecessary python_sitearch macro declaration

http://www.felix-schwarz.name/files/misc/2008/pypdf/1.10-3/pyPdf-1.10-3.fc8.src.rpm
http://www.felix-schwarz.name/files/misc/2008/pypdf/1.10-3/pyPdf.spec


Comment 4 Timothy Selivanow 2008-03-28 21:56:37 UTC
Everything looks good.  No rpmlint output.  Builds fine on Koji for dist-f9
<http://koji.fedoraproject.org/koji/taskinfo?taskID=536803>.

Comment 5 manuel wolfshant 2008-03-28 22:18:56 UTC
well done, Kairo, please proceed with
http://fedoraproject.org/wiki/PackageMaintainers/CVSAdminProcedure#head-1ef2a3bc00349ad095c689ab7caf283db0e2d723

Comment 6 manuel wolfshant 2008-03-28 22:19:21 UTC
sorry for the noise, wrong bz ticket

Comment 7 Paul Howarth 2008-04-01 10:38:18 UTC
Review:

- rpmlint clean
- package naming satisfies python module naming guidelines
- spec file name matches package name
- package meets packaging guidelines
- license is BSD, matches package
- license text not in separate file, but included directly in python source
  files, which are of course packaged and thus license texts are included in
  the package
- spec file written in English and is legible
- sources match upstream (md5sum e15eca1a3ed4d5c0d86370784e552a92)
- package builds OK in mock on i386 and x86_64 for Fedora 9
- buildreqs OK
- no shared libraries, static libraries, header files, pkgconfig files, or
  locale files to worry about
- package doesn't claim to be relocatable
- directory ownership OK
- no duplicate files
- %defattr(...) present and correct in %files section
- %clean section present and correct
- %install section properly cleans buildroot first
- macro usage is consistent
- code, not content
- docs don't affect runtime
- not a GUI app, no desktop file needed
- filenames are all ASCII
- I haven't tested that the package functions OK as it's basically a library
  for use with other apps
- no scriptlets or subpackages
- no file dependencies

Suggestions:

Use a more specific files list, e.g.
%{python_sitelib}/pyPdf*
this helps catch future changes that create extra files in the package, which
you might want to document further in the changelog etc.

Include CHANGELOG in %doc


Comment 8 Toshio Kuratomi 2008-04-01 14:41:17 UTC
One other strenuous suggestion: Please query upstream to include a LICENSE file
in the package.  Especially with the BSD license that has several variants of
which one can be problematic ("BSD with advertising"), this can be important.

Comment 9 Paul Howarth 2008-04-01 14:51:23 UTC
(In reply to comment #8)
> One other strenuous suggestion: Please query upstream to include a LICENSE file
> in the package.  Especially with the BSD license that has several variants of
> which one can be problematic ("BSD with advertising"), this can be important.

The license text for this package is actually included in (some of) the source
files, and is the 3-clause BSD license without the advertising clause. It may
not be possible to have a single license file because not all of the files have
the same copyright holders.

Comment 10 Felix Schwarz 2008-04-01 21:22:46 UTC
Update the package according to comment #7. I can ask upstream if a license file
can be included in the upstream tarball but I hope this package can be included
in Fedora even without this.

Updated files at: http://www.felix-schwarz.name/files/misc/2008/pypdf/1.10-4/


Comment 11 Paul Howarth 2008-04-01 22:20:49 UTC
(In reply to comment #10)
> Update the package according to comment #7. I can ask upstream if a license file
> can be included in the upstream tarball but I hope this package can be included
> in Fedora even without this.

Please do; it's not a blocker though.

> Updated files at: http://www.felix-schwarz.name/files/misc/2008/pypdf/1.10-4/

Approved. You can apply for cvsextras membership in the accounts system now.

Comment 12 Felix Schwarz 2008-04-02 11:58:19 UTC
New Package CVS Request
=======================
Package Name: pyPdf
Short Description: PDF toolkit
Owners: fschwarz
Branches: F-8 F-9
InitialCC: 
Cvsextras Commits: yes


Comment 13 Kevin Fenzi 2008-04-02 17:58:56 UTC
cvs done.

Comment 14 Felix Schwarz 2008-04-13 17:50:36 UTC
pyPdf imported into CVS, builds well (and should be pushed to F-8 soon, see
https://admin.fedoraproject.org/updates/F8/pending/pyPdf-1.10-4.fc8).

Comment 15 Felix Schwarz 2008-05-12 21:30:01 UTC
Package Change Request
======================
Package Name: pyPdf
New Branches: EL-4 EL-5


Comment 16 Kevin Fenzi 2008-05-13 17:07:14 UTC
cvs done.


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