Bug 438892 - Review Request: pyPdf - PDF toolkit
Review Request: pyPdf - PDF toolkit
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Paul Howarth
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-03-25 16:15 EDT by Felix Schwarz
Modified: 2008-05-13 13:07 EDT (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-04-13 13:50:36 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
paul: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Felix Schwarz 2008-03-25 16:15:54 EDT
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 16:28:53 EDT
Rpmlint says:
pyPdf.src: E: invalid-spec-name pypdf.spec
pyPdf.src: W: invalid-license modified BSD

Comment 2 Felix Schwarz 2008-03-25 18:10:18 EDT
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 17:13:21 EDT
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 17:56:37 EDT
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 18:18:56 EDT
well done, Kairo, please proceed with
http://fedoraproject.org/wiki/PackageMaintainers/CVSAdminProcedure#head-1ef2a3bc00349ad095c689ab7caf283db0e2d723
Comment 6 manuel wolfshant 2008-03-28 18:19:21 EDT
sorry for the noise, wrong bz ticket
Comment 7 Paul Howarth 2008-04-01 06:38:18 EDT
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 10:41:17 EDT
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 10:51:23 EDT
(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 17:22:46 EDT
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 18:20:49 EDT
(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 07:58:19 EDT
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 13:58:56 EDT
cvs done.
Comment 14 Felix Schwarz 2008-04-13 13:50:36 EDT
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 17:30:01 EDT
Package Change Request
======================
Package Name: pyPdf
New Branches: EL-4 EL-5
Comment 16 Kevin Fenzi 2008-05-13 13:07:14 EDT
cvs done.

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