Bug 881751 - Review Request: rubygem-pdf-inspector - A tool for analyzing PDF output
Summary: Review Request: rubygem-pdf-inspector - A tool for analyzing PDF output
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Bohuslav "Slavek" Kabrda
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2012-11-29 14:13 UTC by Josef Stribny
Modified: 2016-01-04 05:50 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2012-12-03 15:06:09 UTC
Type: ---
Embargoed:
bkabrda: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Josef Stribny 2012-11-29 14:13:40 UTC
Spec URL: http://data-strzibny.rhcloud.com/rubygem-pdf-inspector.spec
SRPM URL: http://data-strzibny.rhcloud.com/rubygem-pdf-inspector-1.0.2-1.fc17.src.rpm

Description: This library provides a number of PDF::Reader[0] based tools for use in testing PDF output.  Presently, the primary purpose of this tool is to support the tests found in Prawn[1], a pure Ruby PDF generation library.
However, it may be useful to others, so we have made it available as a gem in
its own right.
[0] https://github.com/yob/pdf-reader
[1] https://github.com/sandal/prawn

Fedora Account System Username: jstribny
Koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=4740723

Note: The gem doesn't come with a test suite, tested only using rpmlint, koji and mock --shell

Comment 1 Bohuslav "Slavek" Kabrda 2012-11-30 06:44:35 UTC
I'll take this for a review.

Comment 2 Bohuslav "Slavek" Kabrda 2012-11-30 07:19:33 UTC
- rpmlint gives me:
"incorrect-fsf-address /usr/share/gems/gems/pdf-inspector-1.0.2/GPLv2", which means that the LICENSE file is probably older. Although this is not a showstopper, you should contact the upstream (or ideally send a pull request) to fix this.
- Since this is a pure ruby Gem, it is not optimal to have runtime dependency on Ruby. When we have more Ruby interpreters (e.g. JRuby), this gem should also be usable with these. So please remove Requires: ruby, keeping only the runtime requirement on ruby(abi).
- Please check the wordwrapping in description, it seems to be a little weird :) (word "testing" is on its own line, etc.)

Comment 3 Josef Stribny 2012-12-03 09:41:44 UTC
Hi,

I informed upstream and submitted a pull request [1], deleted the requirement of Ruby and fixed the bad word wrapping.

Spec URL: http://data-strzibny.rhcloud.com/rubygem-pdf-inspector.spec
SRPM URL: http://data-strzibny.rhcloud.com/rubygem-pdf-inspector-1.0.2-1.fc17.src.rpm

[1] https://github.com/prawnpdf/pdf-inspector/pull/8

Comment 4 Bohuslav "Slavek" Kabrda 2012-12-03 09:48:01 UTC
Great, the package looks good now. Only a minor issue: It is customary to bump the release if doing changes during review. So please bump the release and move the two changes you made from release 1 to release 2 before committing.

APPROVED.

Comment 5 Josef Stribny 2012-12-03 10:47:41 UTC
New Package SCM Request
=======================
Package Name: rubygem-pdf-inspector
Short Description: A tool for analyzing PDF output
Owners: jstribny
Branches:
InitialCC:

Comment 6 Gwyn Ciesla 2012-12-03 13:01:46 UTC
Git done (by process-git-requests).


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