Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 175291 - Review Request: perl-PatchReader - utilities to read and manipulate patches and CVS
Summary: Review Request: perl-PatchReader - utilities to read and manipulate patches a...
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Ville Skyttä
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2005-12-08 15:40 UTC by Paul W. Frields
Modified: 2010-07-02 17:12 UTC (History)
0 users

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2005-12-13 12:24:54 UTC
Type: ---
Embargoed:
j: fedora-cvs+


Attachments (Terms of Use)
Cosmetic changes (594 bytes, patch)
2005-12-11 18:59 UTC, Ville Skyttä
no flags Details | Diff

Description Paul W. Frields 2005-12-08 15:40:30 UTC
Spec Name or Url: http://rpm.frields.org/extras-testing/perl-PatchReader/perl-PatchReader.spec
SRPM Name or Url: http://rpm.frields.org/extras-testing/perl-PatchReader/perl-PatchReader-0.9.5-1.src.rpm

Description:
PatchReader is a set of utilities for reading in, transforming, and doing
various other things with a patch.  It basically allows you to create a
chain of readers that can read a patch, remove files from a patch, add
CVS context, fix up the patch root according to CVS, and output the patch
as raw unified or through a template processor (used in some places to
output a patch as HTML).


(As noted in %changelog, the spec is based on Dag Wieers' package of the same name.)

Comment 1 Ville Skyttä 2005-12-08 19:42:13 UTC
Needswork'n'nitpicks:

- Uselessly redefines %{perl_vendor*}, doesn't need %{perl_vendorarch}
- Requires: perl(:MODULE_COMPAT_*) missing
- Doesn't run test suite during build
- Redundant %doc for man page
- Installs *.pm executable
- Off sync with many "best practices" from fedora-rpmdevtools' perl spec
  template

Here's my package which should be ready to roll; "|| :" could be removed from
%check and BuildRequires trimmed assuming recent perl packages though:
http://cachalot.mine.nu/4/SRPMS/perl-PatchReader-0.9.5-0.2.src.rpm

Note: the URLs you posted apparently use some kind of a frame hack which makes
them non-wgettable. 

Comment 2 Paul W. Frields 2005-12-08 21:32:53 UTC
Thanks, Ville -- some questions follow so I can learn something out of this:

(In reply to comment #1)
> - Requires: perl(:MODULE_COMPAT_*) missing

What does this mean?

> - Installs *.pm executable

You must have picked this up during the few minutes before I fixed this problem
-- the ones up there as of this morning don't have this problem, but thanks.

> - Off sync with many "best practices" from fedora-rpmdevtools' perl spec
>   template

Yeah, I should have probably started there instead of appropriating Dag's and
starting with it.

> Here's my package which should be ready to roll; "|| :" could be removed from
> %check and BuildRequires trimmed assuming recent perl packages though:
> http://cachalot.mine.nu/4/SRPMS/perl-PatchReader-0.9.5-0.2.src.rpm

Do you want me to take this one over then?

> Note: the URLs you posted apparently use some kind of a frame hack which makes
> them non-wgettable. 

Yes, but elinks should work fine if you're stuck with a text browser.

Comment 3 Ville Skyttä 2005-12-08 22:39:39 UTC
(In reply to comment #2)
> > - Requires: perl(:MODULE_COMPAT_*) missing
> 
> What does this mean?

Requires: perl(:MODULE_COMPAT_X.Y.Z) is used to make the package depend on a
version of perl that uses modules from the X.Y.Z-versioned directories (as in
eg. /usr/lib/perl5/vendor_perl/X.Y.Z).  See the perl spec template for an
example how to generate that based on the version of perl the package is built with.
 
> > http://cachalot.mine.nu/4/SRPMS/perl-PatchReader-0.9.5-0.2.src.rpm
> 
> Do you want me to take this one over then?

Take what you feel comfortable maintaining in the future.  But as said, the
above package is IMO in a pretty good shape, and using specfiles based on the
spec template makes it easier for folks to review things.

> > them non-wgettable. 
> 
> Yes, but elinks should work fine if you're stuck with a text browser.

OT: not stuck; wget or curl or direct rpm -Uvh <url-to-src.rpm> just
"integrates" better with the way I work.

Comment 4 Paul W. Frields 2005-12-10 22:53:57 UTC
OK, refer to new spec and SRPM for 0.9.5-2:
http://rpm.frields.org/extras-testing/perl-PatchReader/


Comment 5 Ville Skyttä 2005-12-11 18:59:25 UTC
Created attachment 122110 [details]
Cosmetic changes

Approved with the attached patch containing some final cosmetic improvements
applied (can be done post-import, before the first build).

Comment 6 Christian Iseli 2006-12-31 11:39:36 UTC
Please do not remove the FE-ACCEPT blocker.  Thanks.


Comment 7 Paul W. Frields 2010-07-02 11:43:05 UTC
Package Change Request
======================
Package Name: perl-PatchReader
New Branches: EL-5 EL-6
Owners: pfrields perl-sig
InitialCC:

Please create EPEL branches for this package.

Comment 8 Jason Tibbitts 2010-07-02 17:12:30 UTC
CVS done (by process-cvs-requests.py).


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