Bugzilla will be upgraded to version 5.0 on a still to be determined date in the near future. The original upgrade date has been delayed.
Bug 602587 - Review Request: perl-IPC-Signal - Utility functions dealing with signals
Review Request: perl-IPC-Signal - Utility functions dealing with signals
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Paul Howarth
Fedora Extras Quality Assurance
:
Depends On:
Blocks: 589867 602598
  Show dependency treegraph
 
Reported: 2010-06-10 05:12 EDT by Matthias Runge
Modified: 2015-01-07 06:11 EST (History)
4 users (show)

See Also:
Fixed In Version: perl-IPC-Signal-1.00-2.el5
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2010-06-29 11:32:52 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
paul: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Matthias Runge 2010-06-10 05:12:52 EDT
Spec URL: http://www.matthias-runge.de/fedora/perl-IPC-signal.spec
SRPM URL: http://www.matthias-runge.de/fedora/perl-IPC-signal-1.00-1.fc13.src.rpm
Description: 
This Perl module contains utility functions for dealing with signals.
Currently these are just translating between signal names and signal
numbers and vice versa.
Comment 1 Matthias Runge 2010-06-10 05:14:15 EDT
[mrunge@mrungexp SPECS]$ rpmlint ../RPMS/noarch/perl-IPC-signal-1.00-1.fc13.noarch.rpm ../SRPMS/perl-IPC-signal-1.00-1.fc13.src.rpm perl-IPC-signal.spec 
perl-IPC-signal.noarch: W: spelling-error %description -l en_US versa -> avers, verse, verso
perl-IPC-signal.src: W: spelling-error %description -l en_US versa -> avers, verse, verso
2 packages and 1 specfiles checked; 0 errors, 2 warnings.


I think, "vice versa" is correct, the hint to change versa to something else is wrong.
Comment 2 Paul Howarth 2010-06-10 06:03:22 EDT
A few quick comments on the spec to start with:

The README has the usual license statement:
 "All rights reserved.  This program is free software; you can
  redistribute it and/or modify it under the same terms as Perl
  itself."
That makes the license "GPL+ or Artistic", not "Artistic 2.0".

You can drop the boilerplate comments from the spec template, and should take notice of them too: this is a noarch package so including `OPTIMIZE="$RPM_OPT_FLAGS"' is redundant, as is the `find' of empty `.bs' files.

Use an upstream release location for Source0, not a copy from another downstream distribution's ftp server. I suggest one of these:
http://www.cpan.org/modules/by-module/IPC/IPC-Signal-%{version}.tar.gz
http://www.cpan.org/modules/by-module/IPC/ROSCH/IPC-Signal-%{version}.tar.gz
http://search.cpan.org/CPAN/authors/id/R/RO/ROSCH/IPC-Signal-%{version}.tar.gz

I would be more specific with the %files list; rather than:
%{perl_vendorlib}/*
%{_mandir}/man3/*.3*
I would have:
%{perl_vendorlib}/IPC/
%{_mandir}/man3/IPC::Signal.3pm*
This form make it easier to spot significant changes in the package's contents in any future versions but it's not a blocker - your original format is still acceptable if that's what you prefer.

Include Changes and README in %doc. The latter is particularly important as it contains the terms under which the package is licensed.

I agree with you about the rpmlint complaint.
Comment 3 Matthias Runge 2010-06-10 06:25:32 EDT
Thank you for your quick comments. Obviously, you're right, I changed the spec, as you suggested.

Updated versions are under
Spec URL: http://www.matthias-runge.de/fedora/perl-IPC-signal.spec
SRPM URL:
http://www.matthias-runge.de/fedora/perl-IPC-signal-1.00-2.fc13.src.rpm
Comment 4 Paul Howarth 2010-06-10 06:35:04 EDT
You changed the "URL" tag (which was OK) instead of the "Source0" tag (which wasn't), and the comment about possibly using Module::Build is still there...
Comment 5 Matthias Runge 2010-06-10 06:50:51 EDT
fixed.

Updated versions are under
Spec URL: http://www.matthias-runge.de/fedora/perl-IPC-signal.spec
SRPM URL: http://www.matthias-runge.de/fedora/perl-IPC-signal-1.00-3.fc13.src.rpm
Comment 6 Paul Howarth 2010-06-10 11:10:19 EDT
Review Checklist:
- rpmlint OK
- package and spec file naming OK
- package meets guidelines
- license is same as perl, whcih is OK for Fedora
- package license tag is correct: GPL+ or Artistic
- no license text file to include
- spec file is legible and written in English
- source matches upstream
- package builds OK in mock for rawhide x86_64
- buildreqs OK
- no locale data to include
- no shared or static libraries included
- package not intended to be relocatable
- directory owndership is OK
- no duplicate files
- %defattr(...) present and correct
- macro usage is consistent
- code, not content
- no large docs
- docs don't affect runtime
- no subpackages
- not a GUI app so no desktop file needed
- filenames are all ASCII
- no scriptlets
- requires and provides are OK
- manpage provided

Source SHA-256 hashes:
7c21f9c8c2d0c0f0f0f46e77de7c3d879dd562668ddf0525875c38cef2076fd0  Upstream IPC-Signal-1.00.tar.gz
7c21f9c8c2d0c0f0f0f46e77de7c3d879dd562668ddf0525875c38cef2076fd0  Package  IPC-Signal-1.00.tar.gz

Test suite:
$ make test
PERL_DL_NONLAZY=1 /usr/bin/perl "-MExtUtils::Command::MM" "-e" "test_harness(0, 'blib/lib', 'blib/arch')" t/*.t
t/signal.t .. ok
All tests successful.
Files=1, Tests=7,  0 wallclock secs ( 0.02 usr +  0.00 sys =  0.02 CPU)
Result: PASS

Provides:
perl(IPC::Signal) = 1.00
perl-IPC-signal = 1.00-3.fc14

Requires:
perl(:MODULE_COMPAT_5.10.1)  
rpmlib(FileDigests) <= 4.6.0-1
rpmlib(PayloadFilesHavePrefix) <= 4.0-1
rpmlib(CompressedFileNames) <= 3.0.4-1
rpmlib(VersionedDependencies) <= 3.0.3-1
perl >= 0:5.003_94
perl(Exporter)  
perl(strict)  
perl(vars)  
rpmlib(VersionedDependencies) <= 3.0.3-1
rpmlib(PayloadIsXz) <= 5.2-1

rpmlint:
perl-IPC-signal.noarch: W: spelling-error %description -l en_US versa -> avers, verse, verso
perl-IPC-signal.src: W: spelling-error %description -l en_US versa -> avers, verse, verso
2 packages and 0 specfiles checked; 0 errors, 2 warnings.

CPAN RT:
One outstanding issue, which is a feature request rather than a bug.
https://rt.cpan.org/Public/Bug/Display.html?id=11464

Nits:
- please maintain the timestamp of the upstream tarball (use wget or spectool
  to download it - it's dated 1998!)
- a better URL for the upstream project would be:
  http://search.cpan.org/dist/IPC-Signal/
  since it's not version or author specific


Nothing to worry about then. The nits aren't blockers but you might fix them anyway.

APPROVED.
Comment 7 Iain Arnell 2010-06-10 13:39:44 EDT
Paul, sorry to step on your toes, but I'm resetting approval.

There's one blocker. The package name doesn't follow our convention of matching the CPAN's use of upper/lower case. (Strangely, though, I couldn't find this codified anywhere in packaging guidelines).

Please rename this package to perl-IPC-Signal with an uppercase letter S.

(and Matthias, same applies to your other review requests for perl modules - they should all be named like perl-Upstream-DistributionName)
Comment 8 Iain Arnell 2010-06-10 13:44:13 EDT
Typical - no sooner do I hit the button and I remember where it's written (though it could be a bit more explicit about matching case rather than relying on examples to make the point). 

http://fedoraproject.org/wiki/Packaging:NamingGuidelines#Addon_Packages_.28perl_modules.29
Comment 9 Paul Howarth 2010-06-10 14:46:14 EDT
Iain's quite right, don't know how I missed that.

Let's have another cycle round the loop and we can re-approve.
Comment 10 Matthias Runge 2010-06-10 15:06:00 EDT
Understood and thank you Iain, for pointing this out.

Assuming, we do not need to close and to reopen another bug, I've just changed the name. Is this sufficient?

Spec URL: http://matthias-runge.de/fedora/perl-IPC-Signal.spec
SRPM URL: http://matthias-runge.de/fedora/perl-IPC-Signal-1.00-1.fc13.src.rpm
Comment 11 Paul Howarth 2010-06-10 16:14:07 EDT
All fixed. APPROVED again.
Comment 12 Matthias Runge 2010-06-11 02:54:27 EDT
New Package CVS Request
=======================
Package Name: perl-IPC-Signal
Short Description: Utility functions dealing with signals
Owners: mrunge
Branches: F13  F14
Comment 13 Matthias Runge 2010-06-11 02:55:44 EDT
Paul, thanks for your review and effort.
Comment 14 Kevin Fenzi 2010-06-14 00:46:34 EDT
CVS done (by process-cvs-requests.py).

We aren't doing F-14 branches yet.
Comment 15 Fedora Update System 2010-06-14 04:17:06 EDT
perl-IPC-Signal-1.00-1.fc13 has been submitted as an update for Fedora 13.
http://admin.fedoraproject.org/updates/perl-IPC-Signal-1.00-1.fc13
Comment 16 Fedora Update System 2010-06-14 13:10:41 EDT
perl-IPC-Signal-1.00-1.fc13 has been pushed to the Fedora 13 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update perl-IPC-Signal'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/perl-IPC-Signal-1.00-1.fc13
Comment 17 Fedora Update System 2010-06-29 11:32:45 EDT
perl-IPC-Signal-1.00-1.fc13 has been pushed to the Fedora 13 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 18 Matthias Runge 2010-07-04 16:06:33 EDT
Package Change Request
======================
Package Name: perl-IPC-Signal
New Branches: EL-6
Owners: mrunge


I've been asked to include perl-Proc-WaitStat into EL-6. perl-IPC-Signal is a dependency, so it's needed, too.
Comment 19 Kevin Fenzi 2010-07-04 21:43:52 EDT
CVS done (by process-cvs-requests.py).
Comment 20 Matthias Runge 2010-09-23 05:34:47 EDT
Package Change Request
======================
Package Name: perl-IPC-Signal
New Branches: EL-5
Owners: mrunge
Comment 21 Kevin Fenzi 2010-09-25 00:51:49 EDT
Git done (by process-git-requests).
Comment 22 Fedora Update System 2010-09-28 02:35:40 EDT
perl-IPC-Signal-1.00-2.el5 has been submitted as an update for Fedora EPEL 5.
https://admin.fedoraproject.org/updates/perl-IPC-Signal-1.00-2.el5
Comment 23 Fedora Update System 2010-10-14 14:24:04 EDT
perl-IPC-Signal-1.00-2.el5 has been pushed to the Fedora EPEL 5 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 24 Matthias Runge 2015-01-06 03:26:50 EST
Package Change Request
======================
Package Name: perl-IPC-Signal
New Branches: EPEL7
Owners: mrunge
Comment 25 Gwyn Ciesla 2015-01-07 06:11:27 EST
Git done (by process-git-requests).

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