Bug 903829 - (perl-Time-Interval) Review Request: perl-Time-Interval - Convert seconds to human readable form
Review Request: perl-Time-Interval - Convert seconds to human readable form
Status: CLOSED CURRENTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
unspecified Severity medium
: ---
: ---
Assigned To: Petr Šabata
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2013-01-24 16:49 EST by Normunds
Modified: 2014-04-03 12:17 EDT (History)
3 users (show)

See Also:
Fixed In Version: perl-Time-Interval-1.232-1.fc18
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2014-04-03 12:17:37 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
psabata: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Normunds 2013-01-24 16:49:35 EST
Spec URL: http://unibackup.rule.lv/FedoraRPM/perl-Time-Interval.spec
SRPM URL: http://unibackup.rule.lv/FedoraRPM/perl-Time-Interval-1.22-2.fc16.src.rpm
Description: This is a rather simple perl module for dealing with time intervals. Among other things, this module can tell you the number of hours, minutes, and seconds elapsed between two dates.
Fedora Account System Username: normunds
Comment 1 Normunds 2013-01-24 16:57:26 EST
Bug 903829, Bug 903826, Bug 903824 are my first Fedora packages, yet more to come. I checked them with both Mock and Koji for all Fedora releases (16, 17, 18, 19, rawhide).
Comment 2 Normunds 2013-01-26 05:21:06 EST
All packages mentioned below were tested with rpmlint, mock (for i386 arch) and koji (16, 17, 18, 19, rawhide). These are my first packages for Fedora, so if you find something to improve in one of them, don't bother, I'll check other packages for reported problems.

Need sponsor.

bug 903824 perl-Convert-Age.spec
bug 903826 perl-Net-Domain-TLD.spec
bug 903829 perl-Time-Interval.spec
bug 904328 perl-Config-ApacheFormat.spec
bug 904329 perl-Data-Validate-Domain.spec
bug 904330 perl-Data-Validate-IP.spec
bug 904331 perl-Shell.spec

Thanks.
Comment 3 Petr Šabata 2013-01-28 08:09:00 EST
Taking the review.
Comment 4 Petr Šabata 2013-01-28 08:23:52 EST
Issues:

Missing build-time dependencies:
perl(Exporter)
perl(Test)

Lines 30 and 31 are useless.  None of the files has executable bits set.
Line 32 is wrong.  Modules aren't scripts and aren't supposed to contain shebangs.
Line 43 may be removed.  Done by rpmbuild.

Preferrably substitute command macros with simple calls.

The license is not mentioned anywhere in the upstream tarball, not even on the CPAN module webpage.  Please, ask upstream for clarification.
Comment 5 Normunds 2013-01-28 17:01:50 EST
Spec file and src package updated.

 * Fixed dependencies.
 * Replaced macros with simple commands.
 * Removed useless lines
 * Sent query to Time-Interval maintainer. Interestingly, someone has asked this same question to him few years ago, although he looks active other Perl package maintainer, Time-Interval license still is not updated :) 

New package overwritten in original location.
Comment 6 Petr Šabata 2013-01-29 09:54:28 EST
Removing FE-NEEDSPONSOR.
Comment 7 Petr Šabata 2013-01-29 10:52:03 EST
Seems okay now (just the perl macro on the MODULE_COMPAT line...).

(In reply to comment #5)
>  * Sent query to Time-Interval maintainer. Interestingly, someone has asked
> this same question to him few years ago, although he looks active other Perl
> package maintainer, Time-Interval license still is not updated :) 

I see
https://rt.cpan.org/Public/Bug/Display.html?id=43024

I can't approve the review until the licensing issue is cleared up.
Comment 8 Normunds 2013-01-31 17:04:38 EST
Package updated, license is now included in upstream, other requested changes done.

Spec URL: http://unibackup.rule.lv/FedoraRPM/perl-Time-Interval.spec
SRPM URL: http://unibackup.rule.lv/FedoraRPM/perl-Time-Interval-1.231-1.fc16.src.rpm
Comment 9 Petr Šabata 2013-02-01 04:22:36 EST
There's been a new release, 1.232, which removes the ._* files from the distribution.

Two more things:
1. Include the LICENSE file in your %doc
2. Your changelog entry is suddenly without a contact e-mail
Comment 10 Normunds 2013-02-01 04:54:54 EST
Question: is contact e-mail mandatory? I decided that including e-mail in publicly available spec file would be perfect catch for spam bots.

Sorry for inconsistencies.

Thanks.
Comment 11 Petr Šabata 2013-02-01 06:14:40 EST
(In reply to comment #10)
> Question: is contact e-mail mandatory? I decided that including e-mail in
> publicly available spec file would be perfect catch for spam bots.

Yes, it is.  But don't worry too much, your e-mail is available in the publicly available git commits anyway :)
Comment 12 Normunds 2013-02-02 13:44:40 EST
Big thanks for your support with getting response from upstream.

Updated to version 1.232, added obfuscated e-mail, added LICENSE to doc.

Spec URL: http://unibackup.rule.lv/FedoraRPM/perl-Time-Interval.spec
SRPM URL: http://unibackup.rule.lv/FedoraRPM/perl-Time-Interval-1.232-1.fc16.src.rpm

Thanks.
Comment 13 Petr Šabata 2013-02-04 10:09:07 EST
Again, if you must use this format, enclose it with qw(< >) at least.
Comment 14 Normunds 2013-02-04 11:24:05 EST
Spec file/srpm updated, added <> to e-mail.
Comment 15 Normunds 2013-02-05 13:52:20 EST
Another reviewer suggested to use simpler e-mail obfuscation, so, to be consistent, I changed it for all packages.
Comment 16 Petr Šabata 2013-02-06 11:26:02 EST
Alright, approving.
Comment 17 Normunds 2013-02-06 14:41:08 EST
Thanks for your help and support :)
Comment 18 Normunds 2013-02-06 14:46:28 EST
New Package SCM Request
=======================
Package Name: perl-Time-Interval
Short Description: Perl module that converts time intervals of days, hours, minutes, and seconds
Owners: normunds psabata
Branches: f16 f17 f18
InitialCC: perl-sig
Comment 19 Jon Ciesla 2013-02-06 15:06:24 EST
Git done (by process-git-requests).
Comment 20 Fedora Update System 2013-02-14 16:37:22 EST
perl-Time-Interval-1.232-1.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/perl-Time-Interval-1.232-1.fc17
Comment 21 Fedora Update System 2013-02-14 17:14:09 EST
perl-Time-Interval-1.232-1.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/perl-Time-Interval-1.232-1.fc18
Comment 22 Fedora Update System 2013-02-25 21:29:13 EST
perl-Time-Interval-1.232-1.fc17 has been pushed to the Fedora 17 stable repository.
Comment 23 Fedora Update System 2013-02-25 21:40:53 EST
perl-Time-Interval-1.232-1.fc18 has been pushed to the Fedora 18 stable repository.

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