Bug 963197 - Review Request: perl-Mozilla-PublicSuffix - Helper module for using the Mozilla Public Suffix List
Review Request: perl-Mozilla-PublicSuffix - Helper module for using the Mozi...
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Petr Pisar
Fedora Extras Quality Assurance
:
Depends On:
Blocks: 963213
  Show dependency treegraph
 
Reported: 2013-05-15 07:26 EDT by Yanko Kaneti
Modified: 2013-05-27 22:21 EDT (History)
3 users (show)

See Also:
Fixed In Version: perl-Mozilla-PublicSuffix-0.1.15-1.20130516.fc19
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2013-05-27 22:21:14 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
ppisar: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Yanko Kaneti 2013-05-15 07:26:50 EDT
Spec URL: http://declera.com/~yaneti/perl-Mozilla-PublicSuffix/perl-Mozilla-PublicSuffix.spec
SRPM URL: http://declera.com/~yaneti/perl-Mozilla-PublicSuffix/perl-Mozilla-PublicSuffix-0.1.13-1.20130515.fc20.src.rpm
Description: Helper module for using the Mozilla Public Suffix List
Fedora Account System Username: yaneti

This module provides a single function that returns the public suffix of a
domain name by referencing a parsed copy of Mozilla's Public Suffix List.
From the official website at http://publicsuffix.org

Includes an uptodate version of the list as of the time of this submission. The list itself in this version of the module is parsed and then included in the module source, so currently it wouldn't be practical to separate it in a separate data package. Might happen in future versions.
Comment 1 Petr Pisar 2013-05-16 09:26:47 EDT
URL and Source0 are usable. Ok.
Source tar ball is original (SHA-256: b45b1effeaec9b20e8d024c77da2e403b0f89f0444a6c1981dc79aa25b80ced9). Ok.
Summary verified from lib/Mozilla/PublicSuffix.pm. Ok.
Description verified from lib/Mozilla/PublicSuffix.pm. Ok.

TODO: Append trailing slash to the URL. HTTP URL has mandatory path part.

License verified from LICENSE and lib/Mozilla/PublicSuffix.pm. Ok.
No XS code, noArch BuildArch is Ok.

TODO: Do not package dist.ini and perlcritic.rc files. There are no unique data usable for a user.

TODO: Comment source of the overlay effective_tld_names.dat file (Source1).

TODO: You can use plain `perl' command instead of `%{__perl}' macro.
TODO: You can remove deleting empty directories from %install section.

FIX: Build-require `perl(HTTP::Tiny)' (Build.PL:7).

TODO: Build-require `perl(Tie::File)' (Build.PL:9).

FIX: Build-require `perl(Exporter)' (lib/Mozilla/PublicSuffix.pm:6).
FIX: Build-require `perl(File::Temp)' (t/00-compile.t:11).

All tests pass. Ok.

$ rpmlint perl-Mozilla-PublicSuffix.spec ../SRPMS/perl-Mozilla-PublicSuffix-0.1.13-1.20130515.fc20.src.rpm ../RPMS/noarch/perl-Mozilla-PublicSuffix-0.1.13-1.20130515.fc20.noarch.rpm 
2 packages and 1 specfiles checked; 0 errors, 0 warnings.
rpmlint is Ok.

Package does not build in F20 (http://koji.fedoraproject.org/koji/taskinfo?taskID=5388055). Ok.

$ rpm -q -lv -p ../RPMS/noarch/perl-Mozilla-PublicSuffix-0.1.13-1.20130515.fc20.noarch.rpm 
drwxr-xr-x    2 root    root                        0 May 16 15:02 /usr/share/doc/perl-Mozilla-PublicSuffix-0.1.13
-rw-r--r--    1 root    root                     2493 Mar  6 22:22 /usr/share/doc/perl-Mozilla-PublicSuffix-0.1.13/Changes
-rw-r--r--    1 root    root                     1162 Mar  6 22:22 /usr/share/doc/perl-Mozilla-PublicSuffix-0.1.13/LICENSE
-rw-r--r--    1 root    root                      276 Mar  6 22:22 /usr/share/doc/perl-Mozilla-PublicSuffix-0.1.13/README
-rw-r--r--    1 root    root                      885 Mar  6 22:22 /usr/share/doc/perl-Mozilla-PublicSuffix-0.1.13/dist.ini
-rw-r--r--    1 root    root                   101920 May 16 14:41 /usr/share/doc/perl-Mozilla-PublicSuffix-0.1.13/effective_tld_names.dat
-rw-r--r--    1 root    root                      372 Mar  6 22:22 /usr/share/doc/perl-Mozilla-PublicSuffix-0.1.13/perlcritic.rc
-rw-r--r--    1 root    root                     2709 May 16 15:02 /usr/share/man/man3/Mozilla::PublicSuffix.3pm.gz
drwxr-xr-x    2 root    root                        0 May 16 15:02 /usr/share/perl5/vendor_perl/Mozilla
-rw-r--r--    1 root    root                    91435 May 16 15:02 /usr/share/perl5/vendor_perl/Mozilla/PublicSuffix.pm
File permissions and layout are Ok.

$ rpm -q --requires -p ../RPMS/noarch/perl-Mozilla-PublicSuffix-0.1.13-1.20130515.fc20.noarch.rpm | sort | uniq -c
      1 perl(Exporter)
      1 perl(:MODULE_COMPAT_5.16.3)
      1 perl(strict)
      1 perl(URI::_idna)
      1 perl(utf8)
      1 perl(warnings)
      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-Mozilla-PublicSuffix-0.1.13-1.20130515.fc20.noarch.rpm | sort | uniq -c
      1 perl(Mozilla::PublicSuffix) = 0.1.13
      1 perl-Mozilla-PublicSuffix = 0.1.13-1.20130515.fc20
Binary provides are Ok.

$ resolvedeps rawhide ../RPMS/noarch/perl-Mozilla-PublicSuffix-0.1.13-1.20130515.fc20.noarch.rpm 
Binary dependencies resolvable. 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 2 Yanko Kaneti 2013-05-16 17:23:39 EDT
Thanks for the review. New spec should fix all those issues.

0.1.13-2.20130516
- Address review comments. (#963197#c1)

Spec URL: http://declera.com/~yaneti/perl-Mozilla-PublicSuffix/perl-Mozilla-PublicSuffix.spec
SRPM URL: http://declera.com/~yaneti/perl-Mozilla-PublicSuffix/perl-Mozilla-PublicSuffix-0.1.13-2.20130516.fc20.src.rpm
Comment 3 Petr Pisar 2013-05-17 02:30:48 EDT
Spec file changes:

--- perl-Mozilla-PublicSuffix.spec.old  2013-05-15 13:21:25.000000000 +0200
+++ perl-Mozilla-PublicSuffix.spec      2013-05-16 23:23:22.000000000 +0200
@@ -1,13 +1,14 @@
-%global publicsuffix_date 20130515
+%global publicsuffix_date 20130516

 Name:           perl-Mozilla-PublicSuffix
 Version:        0.1.13
-Release:        1.%{publicsuffix_date}%{?dist}
+Release:        2.%{publicsuffix_date}%{?dist}
 Summary:        Get a domain name's public suffix via the Mozilla Public Suffix List
 License:        MIT
 Group:          Development/Libraries
 URL:            http://search.cpan.org/dist/Mozilla-PublicSuffix/
 Source0:        http://www.cpan.org/authors/id/R/RS/RSIMOES/Mozilla-PublicSuffix-v%{version}.tar.gz
+# http://mxr.mozilla.org/mozilla-central/source/netwerk/dns/effective_tld_names.dat?raw=1
 Source1:        effective_tld_names.dat

 BuildArch:      noarch
@@ -16,25 +17,28 @@
 BuildRequires:  perl(Module::Build)
 BuildRequires:  perl(Test::More)
 BuildRequires:  perl(URI::_idna)
+BuildRequires:  perl(HTTP::Tiny)
+BuildRequires:  perl(Tie::File)
+BuildRequires:  perl(Exporter)
+
 Requires:       perl(URI::_idna)
 Requires:       perl(:MODULE_COMPAT_%(eval "`%{__perl} -V:version`"; echo $version))

 %description
 This module provides a single function that returns the public suffix of a
 domain name by referencing a parsed copy of Mozilla's Public Suffix List.
-From the official website at http://publicsuffix.org
+From the official website at http://publicsuffix.org/

 %prep
 %setup -q -n Mozilla-PublicSuffix-v%{version}
 cp -a %{SOURCE1} ./

 %build
-%{__perl} Build.PL installdirs=vendor </dev/null
+perl Build.PL installdirs=vendor </dev/null
 ./Build

 %install
 ./Build install destdir=$RPM_BUILD_ROOT create_packlist=0
-find $RPM_BUILD_ROOT -depth -type d -exec rmdir {} 2>/dev/null \;

 %{_fixperms} $RPM_BUILD_ROOT/*

@@ -42,10 +46,13 @@
 ./Build test

 %files
-%doc Changes dist.ini effective_tld_names.dat LICENSE perlcritic.rc README
+%doc Changes effective_tld_names.dat LICENSE README
 %{perl_vendorlib}/*
 %{_mandir}/man3/*

 %changelog
+* Thu May 16 2013 Yanko Kaneti <yaneti@declera.com> 0.1.13-2.20130516
+- Address review comments (#963197#1)
+
 * Wed May 15 2013 Yanko Kaneti <yaneti@declera.com> 0.1.13-1.20130515
 - Specfile autogenerated by cpanspec 1.78 and modified for review


> TODO: Append trailing slash to the URL. HTTP URL has mandatory path part.
-From the official website at http://publicsuffix.org
+From the official website at http://publicsuffix.org/
Ok.

> TODO: Do not package dist.ini and perlcritic.rc files. There are no unique data
> usable for a user.
-%doc Changes dist.ini effective_tld_names.dat LICENSE perlcritic.rc README
+%doc Changes effective_tld_names.dat LICENSE README
Ok.

> TODO: Comment source of the overlay effective_tld_names.dat file (Source1).
+# http://mxr.mozilla.org/mozilla-central/source/netwerk/dns/effective_tld_names.dat?raw=1
Ok.

> TODO: You can use plain `perl' command instead of `%{__perl}' macro.
-%{__perl} Build.PL installdirs=vendor </dev/null
+perl Build.PL installdirs=vendor </dev/null
Ok.
TODO: The same applies to perl(:MODULE_COMPAT definition.

> TODO: You can remove deleting empty directories from %install section.
-find $RPM_BUILD_ROOT -depth -type d -exec rmdir {} 2>/dev/null \;
Ok.

> FIX: Build-require `perl(HTTP::Tiny)' (Build.PL:7).
+BuildRequires:  perl(HTTP::Tiny)
Ok.

> TODO: Build-require `perl(Tie::File)' (Build.PL:9).
+BuildRequires:  perl(Tie::File)
Ok.

> FIX: Build-require `perl(Exporter)' (lib/Mozilla/PublicSuffix.pm:6).
+BuildRequires:  perl(Exporter)
Ok.

> FIX: Build-require `perl(File::Temp)' (t/00-compile.t:11).
Ok.

$ rpmlint perl-Mozilla-PublicSuffix.spec ../SRPMS/perl-Mozilla-PublicSuffix-0.1.13-2.20130516.fc20.src.rpm ../RPMS/noarch/perl-Mozilla-PublicSuffix-0.1.13-2.20130516.fc20.noarch.rpm 
2 packages and 1 specfiles checked; 0 errors, 0 warnings.
rpmlint is Ok.

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

Please consider fixing the `TODO' item before building this package.

Resolution: Package APPROVED.
Comment 4 Yanko Kaneti 2013-05-17 02:42:04 EDT
Missed that last TODO. Will fix.

New Package SCM Request
=======================
Package Name: perl-Mozilla-PublicSuffix
Short Description: Get a domain name's public suffix via the Mozilla Public Suffix List
Owners: yaneti
Branches: f19 el6
InitialCC: perl-sig
Comment 5 Gwyn Ciesla 2013-05-17 09:10:52 EDT
Git done (by process-git-requests).
Comment 6 Fedora Update System 2013-05-17 10:31:19 EDT
perl-Mozilla-PublicSuffix-0.1.13-2.20130516.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/perl-Mozilla-PublicSuffix-0.1.13-2.20130516.fc19
Comment 7 Fedora Update System 2013-05-17 18:22:07 EDT
perl-Mozilla-PublicSuffix-0.1.13-2.20130516.fc19 has been pushed to the Fedora 19 testing repository.
Comment 8 Fedora Update System 2013-05-20 16:10:17 EDT
perl-Mozilla-PublicSuffix-0.1.15-1.20130516.fc19 has been pushed to the Fedora 19 testing repository.
Comment 9 Fedora Update System 2013-05-27 22:21:14 EDT
perl-Mozilla-PublicSuffix-0.1.15-1.20130516.fc19 has been pushed to the Fedora 19 stable repository.

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