Bug 881751

Summary:	Review Request: rubygem-pdf-inspector - A tool for analyzing PDF output
Product:	[Fedora] Fedora	Reporter:	Josef Stribny <jstribny>
Component:	Package Review	Assignee:	Bohuslav "Slavek" Kabrda <bkabrda>
Status:	CLOSED NEXTRELEASE	QA Contact:	Fedora Extras Quality Assurance <extras-qa>
Severity:	medium	Docs Contact:
Priority:	medium
Version:	rawhide	CC:	bkabrda, hhorak, notting, package-review
Target Milestone:	---	Flags:	bkabrda: fedora-review+ gwync: fedora-cvs+
Target Release:	---
Hardware:	All
OS:	Linux
Whiteboard:
Fixed In Version:		Doc Type:	Bug Fix
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2012-12-03 15:06:09 UTC	Type:	---
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Embargoed:

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