Bug 1067003 - Review Request: perl-Time-ParseDate - Perl modules for parsing dates and time
Summary: Review Request: perl-Time-ParseDate - Perl modules for parsing dates and time
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Petr Pisar
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2014-02-19 13:50 UTC by Denis Fateyev
Modified: 2014-07-20 17:58 UTC (History)
5 users (show)

Fixed In Version: perl-Time-ParseDate-2013.1113-2.el5
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2014-07-14 00:54:11 UTC
Type: ---
Embargoed:
ppisar: fedora-review+


Attachments (Terms of Use)

Description Denis Fateyev 2014-02-19 13:50:53 UTC
Spec URL: http://www.fateyev.com/RPMS/Fedora20/testing/perl-Time-ParseDate.spec
SRPM URL: http://www.fateyev.com/RPMS/Fedora20/testing/SRPMS/perl-Time-ParseDate-2013.1113-1.fc20.denf.src.rpm
Description: This module recognizes the above date/time formats. Usually a date and a time are specified. There are numerous options for controlling what is
recognized and what is not.
Fedora Account System Username: dfateyev

Koji scratch builds:
https://koji.fedoraproject.org/koji/taskinfo?taskID=6547163 (Rawhide)
https://koji.fedoraproject.org/koji/taskinfo?taskID=6547159 (EPEL6)
https://koji.fedoraproject.org/koji/taskinfo?taskID=6547161 (EPEL 5)

Comment 1 Denis Fateyev 2014-02-19 14:13:36 UTC
Description is a bit bogus (auto-imported from CPAN), here is a more appropriate and shorten version:
This module recognizes various absolute and relative date/time formats. There can be numerous options for controlling what is recognized and what is not.

Also note, that it's licensed under approved TPDL license:
https://fedoraproject.org/wiki/Licensing/TPDL

Comment 2 Petr Pisar 2014-02-26 08:41:49 UTC
This package looks like a replacement for perl-Time-modules. See <
http://search.cpan.org/~muir/Time-modules/> and <http://search.cpan.org/~muir/Time-ParseDate/>

If this is so, you have to coordinate your effort with perl-Time-modules maintainer. You have to add proper Obsoletes statement into perl-Time-ParseDate and perl-Time-ParseDate maintainer should kill the package.

Also please read perl-Time-modules spec file and align your package. Especially the BuildRequires and %doc section. You can also get %description from there. Your proposed package looks very poor.

Comment 3 Xavier Bachelot 2014-03-04 13:27:16 UTC
(In reply to Petr Pisar from comment #2)
> This package looks like a replacement for perl-Time-modules. See <
> http://search.cpan.org/~muir/Time-modules/> and
> <http://search.cpan.org/~muir/Time-ParseDate/>
> 
> If this is so, you have to coordinate your effort with perl-Time-modules
> maintainer. You have to add proper Obsoletes statement into
> perl-Time-ParseDate and perl-Time-ParseDate maintainer should kill the
> package.
> 
> Also please read perl-Time-modules spec file and align your package.
> Especially the BuildRequires and %doc section. You can also get %description
> from there. Your proposed package looks very poor.

This indeed looks like a package rename, and so you must follow the package rename procedure :
https://fedoraproject.org/wiki/Package_Renaming_Process.

Funnily enough, it appears that Petr is asking to coordinate with the package maintainer, which happens to be him ;-) I'm co-maintainer in the Fedora branches and maintainer for the EL5 branch (no sure why there's no EL6 branch...)

As Petr suggested, please update your spec to match the former perl-Time-modules.spec. I'll take care of retiring perl-Time-modules from the EL5 branch when perl-Time-ParseDate is ready and Petr will be the one to do the retirement for the Fedora branches (One need 'approveacls' privileges to retire a package).
Also, please keep me as a co-maintainer in all branches. I assume Petr wants to be kept as (co-)maintainer too for the Fedora branches.

Comment 5 Xavier Bachelot 2014-06-28 20:26:22 UTC
The package looks good, there are only a couple mostly cosmetic issues :
- blank line at top of spec.
- it would be clearer to not mix Requires and BuildRequires. Move the only Requires line below the last BuildRequires line.
- the %defattr line in the %files section is not needed for anything but EL5.

Comment 6 Denis Fateyev 2014-06-28 22:19:02 UTC
Fixed 1 and 2. As for `%defattr`, I'm also planning to package for RHEL5, and kept it since it doesn't break anything on newer versions.
http://www.fateyev.com/RPMS/Fedora20/testing/perl-Time-ParseDate.spec

Comment 7 Paul Howarth 2014-06-29 09:46:28 UTC
(In reply to Denis Fateyev from comment #6)
> Fixed 1 and 2. As for `%defattr`, I'm also planning to package for RHEL5,
> and kept it since it doesn't break anything on newer versions.

It's not needed for EL-5 either actually. EL-4 needed it buts it's redundant since rpm 4.4.

Comment 9 Petr Pisar 2014-07-01 13:16:37 UTC
The standalone spec file and file from source RPM differ. I will use the standalone one for this review.

The URL and Source0 are usable. Ok.
The source archive is original (SHA-256: 14a761a45885cbff907531a0b293a7553100260ec0aa6cb51f4deef75616cdfe). Ok.
Summary is Ok.
Description is Ok.

TODO: Wrap the description paragraph to 80 columns.

License verified from lib/Time/ParseDate.pm, lib/Time/CTime.pm, lib/Time/DaysInMonth.pm, lib/Time/JulianDay.pm, lib/Time/Timezone.pm.

FIX: The lib/Time/Timezone.pm is not TPDL-licensed. It is "Public Domain":

=head1 LICENSE

David Muir Sharnoff disclaims any copyright and puts his contribution
to this module in the public domain.

Change the license tag from (TPDL) to (TPDL and Public Domain).

No XS code presents, noarch BuildArch is Ok.
Build-time dependencies are Ok.
All tests pass. Ok.

$ rpmlint perl-Time-ParseDate.spec ../SRPMS/perl-Time-ParseDate-2013.1113-2.fc21.src.rpm ../RPMS/noarch/perl-Time-ParseDate-2013.1113-2.fc21.noarch.rpm 
perl-Time-ParseDate.src: W: spelling-error %description -l en_US CTime -> C Time, Crime, Clime
perl-Time-ParseDate.src: W: spelling-error %description -l en_US DaysInMonth -> Semimonthly
perl-Time-ParseDate.src: W: spelling-error %description -l en_US JulianDay -> Julian Day, Julian-Day, Julian-day
perl-Time-ParseDate.src: E: description-line-too-long C There can be numerous options for controlling what is recognized and what is not.
perl-Time-ParseDate.noarch: W: spelling-error %description -l en_US CTime -> C Time, Crime, Clime
perl-Time-ParseDate.noarch: W: spelling-error %description -l en_US DaysInMonth -> Semimonthly
perl-Time-ParseDate.noarch: W: spelling-error %description -l en_US JulianDay -> Julian Day, Julian-Day, Julian-day
perl-Time-ParseDate.noarch: E: description-line-too-long C There can be numerous options for controlling what is recognized and what is not.
2 packages and 1 specfiles checked; 2 errors, 6 warnings.
rpmlint is Ok.

$ rpm -q -lv -p ../RPMS/noarch/perl-Time-ParseDate-2013.1113-2.fc21.noarch.rpm 
drwxr-xr-x    2 root    root                        0 Jul  1 15:02 /usr/share/doc/perl-Time-ParseDate
-rw-r--r--    1 root    root                     7275 Nov 14  2013 /usr/share/doc/perl-Time-ParseDate/Changes
-rw-r--r--    1 root    root                      604 Nov 28  1998 /usr/share/doc/perl-Time-ParseDate/README
-rw-r--r--    1 root    root                     2805 Jul  1 15:02 /usr/share/man/man3/Time::CTime.3pm.gz
-rw-r--r--    1 root    root                     2181 Jul  1 15:02 /usr/share/man/man3/Time::DaysInMonth.3pm.gz
-rw-r--r--    1 root    root                     2901 Jul  1 15:02 /usr/share/man/man3/Time::JulianDay.3pm.gz
-rw-r--r--    1 root    root                     3674 Jul  1 15:02 /usr/share/man/man3/Time::ParseDate.3pm.gz
-rw-r--r--    1 root    root                     2321 Jul  1 15:02 /usr/share/man/man3/Time::Timezone.3pm.gz
drwxr-xr-x    2 root    root                        0 Jul  1 15:02 /usr/share/perl5/vendor_perl/Time
-rw-r--r--    1 root    root                     5780 Sep 20  2013 /usr/share/perl5/vendor_perl/Time/CTime.pm
-rw-r--r--    1 root    root                     1330 Apr  1  2008 /usr/share/perl5/vendor_perl/Time/DaysInMonth.pm
-rw-r--r--    1 root    root                     5940 May  6  2011 /usr/share/perl5/vendor_perl/Time/JulianDay.pm
-rw-r--r--    1 root    root                    29907 Nov 14  2013 /usr/share/perl5/vendor_perl/Time/ParseDate.pm
-rw-r--r--    1 root    root                     9604 Sep 20  2013 /usr/share/perl5/vendor_perl/Time/Timezone.pm
File permissions and layout is Ok.

$ rpm -q --requires -p ../RPMS/noarch/perl-Time-ParseDate-2013.1113-2.fc21.noarch.rpm | sort -f | uniq -c
      1 perl >= 0:5.000
      1 perl >= 0:5.002
      1 perl(:MODULE_COMPAT_5.18.2)
      1 perl(Carp)
      1 perl(Exporter)
      1 perl(integer)
      1 perl(strict)
      1 perl(Time::CTime)
      1 perl(Time::JulianDay)
      1 perl(Time::Timezone)
      1 perl(vars)
      1 rpmlib(CompressedFileNames) <= 3.0.4-1
      1 rpmlib(FileDigests) <= 4.6.0-1
      1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1
      1 rpmlib(PayloadIsXz) <= 5.2-1
Binary requires are Ok.

$ rpm -q --provides -p ../RPMS/noarch/perl-Time-ParseDate-2013.1113-2.fc21.noarch.rpm | sort -f | uniq -c
      1 perl(Time::CTime) = 2011.0505
      1 perl(Time::DaysInMonth) = 99.1117
      1 perl(Time::JulianDay) = 2011.0505
      1 perl(Time::ParseDate) = 2013.1113
      1 perl(Time::Timezone) = 2006.0814
      1 perl-Time-modules = 2013.1113-2.fc21
      1 perl-Time-ParseDate = 2013.1113-2.fc21
Binary provides are Ok.

This package replaces perl-Time-modules correctly. Ok.

Package builds in F21 (http://koji.fedoraproject.org/koji/taskinfo?taskID=7096282). Ok.

Otherwise the package is in line with Fedora and Perl packaging guidelines.

Please correct all `FIX' issues, consider fixing `TODO' items, and provide new spec file.
Resolution: Package NOT approved.

Comment 10 Denis Fateyev 2014-07-01 21:54:16 UTC
Fixed the long description and adjusted license tag.
Updated version: http://www.fateyev.com/RPMS/Fedora20/testing/perl-Time-ParseDate.spec

Comment 11 Petr Pisar 2014-07-02 06:10:35 UTC
Spec file changes:

--- perl-Time-ParseDate.spec.old        2014-07-01 02:06:10.000000000 +0200
+++ perl-Time-ParseDate.spec    2014-07-01 23:50:07.000000000 +0200
@@ -3,7 +3,7 @@
 Release:        2%{?dist}
 Summary:        Perl modules for parsing dates and times
 # See https://fedoraproject.org/wiki/Licensing/TPDL
-License:        TPDL
+License:        TPDL and Public Domain
 Group:          Development/Libraries
 URL:            http://search.cpan.org/dist/Time-ParseDate/

@@ -34,7 +34,7 @@
 Time-ParseDate provides several Perl modules, including Time::CTime,
 Time::DaysInMonth, Time::JulianDay, Time::ParseDate, and Time::Timezone.
 These modules can be useful for parsing and manipulating dates and times.
-There can be numerous options for controlling what is recognized and what is not.
+There are numerous options to control what is recognized and what is not.

 %prep
 %setup -q -n Time-ParseDate-%{version}
@@ -69,7 +69,7 @@


-* Fri Jun 27 2014 Denis Fateyev <denis> - 2013.1113-2
+* Tue Jul 01 2014 Denis Fateyev <denis> - 2013.1113-2
 - Specfile improvements and cleanup

 * Tue Feb 18 2014 Denis Fateyev <denis> - 2013.1113-1


> TODO: Wrap the description paragraph to 80 columns.
-There can be numerous options for controlling what is recognized and what is not.
+There are numerous options to control what is recognized and what is not.
Ok.

> FIX: The lib/Time/Timezone.pm is not TPDL-licensed. It is "Public Domain":
 %changelog
-License:        TPDL
+License:        TPDL and Public Domain
Ok.

$ rpmlint perl-Time-ParseDate.spec ../SRPMS/perl-Time-ParseDate-2013.1113-2.fc21.src.rpm ../RPMS/noarch/perl-Time-ParseDate-2013.1113-2.fc21.noarch.rpm 
perl-Time-ParseDate.src: W: spelling-error %description -l en_US CTime -> C Time, Crime, Clime
perl-Time-ParseDate.src: W: spelling-error %description -l en_US DaysInMonth -> Semimonthly
perl-Time-ParseDate.src: W: spelling-error %description -l en_US JulianDay -> Julian Day, Julian-Day, Julian-day
perl-Time-ParseDate.noarch: W: spelling-error %description -l en_US CTime -> C Time, Crime, Clime
perl-Time-ParseDate.noarch: W: spelling-error %description -l en_US DaysInMonth -> Semimonthly
perl-Time-ParseDate.noarch: W: spelling-error %description -l en_US JulianDay -> Julian Day, Julian-Day, Julian-day
2 packages and 1 specfiles checked; 0 errors, 6 warnings.
rpmlint is Ok.

Resolution: Package APPROVED.

Comment 12 Denis Fateyev 2014-07-02 20:46:23 UTC
New Package SCM Request
=======================
Package Name: perl-Time-ParseDate
Short Description: Perl modules for parsing dates and time
Owners: dfateyev ppisar xavierb
Branches: f19 f20 el5 el6 epel7
InitialCC: perl-sig

Comment 13 Gwyn Ciesla 2014-07-03 12:01:06 UTC
Git done (by process-git-requests).

Comment 14 Fedora Update System 2014-07-04 13:48:38 UTC
perl-Time-ParseDate-2013.1113-2.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/perl-Time-ParseDate-2013.1113-2.fc20

Comment 15 Fedora Update System 2014-07-04 13:50:10 UTC
perl-Time-ParseDate-2013.1113-2.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/perl-Time-ParseDate-2013.1113-2.fc19

Comment 16 Fedora Update System 2014-07-04 13:54:17 UTC
perl-Time-ParseDate-2013.1113-2.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/perl-Time-ParseDate-2013.1113-2.el6

Comment 17 Fedora Update System 2014-07-04 13:55:40 UTC
perl-Time-ParseDate-2013.1113-2.el5 has been submitted as an update for Fedora EPEL 5.
https://admin.fedoraproject.org/updates/perl-Time-ParseDate-2013.1113-2.el5

Comment 18 Fedora Update System 2014-07-05 09:33:20 UTC
perl-Time-ParseDate-2013.1113-2.el5 has been pushed to the Fedora EPEL 5 testing repository.

Comment 19 Denis Fateyev 2014-07-07 19:37:27 UTC
Unpushed and deleted update for 'EL6' branch since it conflicts with the base package `perl-Time-modules` (we overlooked this earlier).
See #1116852 for details.

Comment 20 Petr Pisar 2014-07-08 07:24:18 UTC
(In reply to Denis Fateyev from comment #12)
> New Package SCM Request
> =======================
> Package Name: perl-Time-ParseDate
> Short Description: Perl modules for parsing dates and time
> Owners: dfateyev ppisar xavierb
> Branches: f19 f20 el5 el6 epel7
> InitialCC: perl-sig

Rel-engs, could you please add the `perl-sig' pseudouser the watchbugzilla and watchcommits permissions in all branches?

I cannot see the user in the pkgdb-cli output neither on the web interface. I also thing the mailing list linked to the pseudouser did not received any e-mail for this package.

Comment 21 Gwyn Ciesla 2014-07-08 12:48:22 UTC
Complete.

Comment 22 Petr Pisar 2014-07-08 14:36:28 UTC
I've blocked perl-Time-modules in F21.

Comment 23 Fedora Update System 2014-07-14 00:54:11 UTC
perl-Time-ParseDate-2013.1113-2.fc19 has been pushed to the Fedora 19 stable repository.

Comment 24 Fedora Update System 2014-07-14 00:54:50 UTC
perl-Time-ParseDate-2013.1113-2.fc20 has been pushed to the Fedora 20 stable repository.

Comment 25 Fedora Update System 2014-07-20 17:58:56 UTC
perl-Time-ParseDate-2013.1113-2.el5 has been pushed to the Fedora EPEL 5 stable repository.


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