Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=901387
Bug ID: 901387 Summary: Review Request: perl-Text-Affixes - Prefixes and suffixes analysis of text Product: Fedora Version: rawhide Component: Package Review Severity: medium Priority: medium Reporter: bochecha@fedoraproject.org
Spec URL: http://bochecha.fedorapeople.org/packages/perl-Text-Affixes-0.07-1.fc18.src.... SRPM URL: http://bochecha.fedorapeople.org/packages/perl-Text-Affixes.spec
Description: Provides methods for prefixes and suffixes analysis of text.
Fedora Account System Username: bochecha
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=901387
Mathieu Bridon bochecha@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |901419
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=901387
Petr Pisar ppisar@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |ppisar@redhat.com Assignee|nobody@fedoraproject.org |ppisar@redhat.com Flags| |fedora-review?
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=901387
Petr Pisar ppisar@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? | Flags| |fedora-review+
--- Comment #1 from Petr Pisar ppisar@redhat.com --- URL and Source0 are useful. Ok. Source file is original (SHA-256: c771fd9e46d16f2a38901afc1252445d8b586308e4da35dd88983b675b23afa4). Ok. Summary verified from (Affixes.pm). Ok. Description verified from (Affixes.pm). Ok. License verified from (Affixes.pm). Ok. No XS code, noarch BuildArch is Ok.
TODO: You can replace %{__perl} macro with plain perl command. TODO: Replace PERL_INSTALL_ROOT variable with DESTDIR in %install section. Current ExtUtils::MakeMaker understands DESTDIR.
TODO: Build-require `perl(Exporter)' (Affixes.pm:7). TODO: Build-require `perl(AutoLoader)' (Affixes.pm:8).
TODO: Specify version constrain `>= 1.14' at perl(Test::Pod) (t/pod.t:4). TODO: Specify version constrain `>= 1.04' at perl(Test::Pod::Coverage) (t/pod-coverage.t:4).
All test pass. Ok.
$ rpmlint perl-Text-Affixes.spec ../SRPMS/perl-Text-Affixes-0.07-1.fc19.src.rpm ../RPMS/noarch/perl-Text-Affixes-0.07-1.fc19.noarch.rpm 2 packages and 1 specfiles checked; 0 errors, 0 warnings. rpmlint is Ok.
$ rpm -q -lv -p ../RPMS/noarch/perl-Text-Affixes-0.07-1.fc19.noarch.rpm drwxr-xr-x 2 root root 0 Jan 21 17:59 /usr/share/doc/perl-Text-Affixes-0.07 -rw-r--r-- 1 root root 921 Nov 19 2005 /usr/share/doc/perl-Text-Affixes-0.07/Changes -rw-r--r-- 1 root root 3664 Nov 19 2005 /usr/share/doc/perl-Text-Affixes-0.07/README -rw-r--r-- 1 root root 3155 Jan 21 17:59 /usr/share/man/man3/Text::Affixes.3pm.gz drwxr-xr-x 2 root root 0 Jan 21 17:59 /usr/share/perl5/vendor_perl/Text -rw-r--r-- 1 root root 5165 Nov 19 2005 /usr/share/perl5/vendor_perl/Text/Affixes.pm drwxr-xr-x 2 root root 0 Jan 21 17:59 /usr/share/perl5/vendor_perl/auto/Text drwxr-xr-x 2 root root 0 Jan 21 17:59 /usr/share/perl5/vendor_perl/auto/Text/Affixes -rw-r--r-- 1 root root 91 Jan 21 17:59 /usr/share/perl5/vendor_perl/auto/Text/Affixes/autosplit.ix File layout and permissions are Ok.
(I'm not convinced the AutoLoad stuff is needed, but that's upstream problem.)
$ rpm -q --requires -p ../RPMS/noarch/perl-Text-Affixes-0.07-1.fc19.noarch.rpm | sort |uniq -c 1 perl >= 0:5.006 1 perl(AutoLoader) 1 perl(Exporter) 1 perl(:MODULE_COMPAT_5.16.2) 1 perl(strict) 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-Text-Affixes-0.07-1.fc19.noarch.rpm | sort |uniq -c 1 perl(Text::Affixes) = 0.07 1 perl-Text-Affixes = 0.07-1.fc19 Binary provides are Ok.
$ resolvedeps rawhide ../RPMS/noarch/perl-Text-Affixes-0.07-1.fc19.noarch.rpm Binary dependencies resolvable. Ok.
Package builds in F19 (http://koji.fedoraproject.org/koji/taskinfo?taskID=4890237). Ok.
Package is in line with Fedora and Perl packaging guidelines.
Please consider fixing `TODO' items before building this package. Resolution: Package APPROVED.
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=901387
--- Comment #2 from Mathieu Bridon bochecha@fedoraproject.org --- (In reply to comment #1)
TODO: You can replace %{__perl} macro with plain perl command.
Fixed.
TODO: Replace PERL_INSTALL_ROOT variable with DESTDIR in %install section. Current ExtUtils::MakeMaker understands DESTDIR.
Out of curiosity, do you know what is the minimal version of ExtUtils::MakeMaker which started understanding DESTDIR? (to avoid closing the door to EPEL builds, in this package or in my future submissions)
TODO: Build-require `perl(Exporter)' (Affixes.pm:7). TODO: Build-require `perl(AutoLoader)' (Affixes.pm:8).
Fixed.
TODO: Specify version constrain `>= 1.14' at perl(Test::Pod) (t/pod.t:4). TODO: Specify version constrain `>= 1.04' at perl(Test::Pod::Coverage) (t/pod-coverage.t:4).
Is that really necessary?
I mean, even EPEL 4 has recent enough versions...
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=901387
--- Comment #3 from Petr Pisar ppisar@redhat.com --- (In reply to comment #2)
(In reply to comment #1)
TODO: You can replace %{__perl} macro with plain perl command.
Fixed.
Updated spec file is on the same address?
TODO: Replace PERL_INSTALL_ROOT variable with DESTDIR in %install section. Current ExtUtils::MakeMaker understands DESTDIR.
Out of curiosity, do you know what is the minimal version of ExtUtils::MakeMaker which started understanding DESTDIR? (to avoid closing the door to EPEL builds, in this package or in my future submissions)
According to ExtUtils::MakeMaker changelog http://cpansearch.perl.org/src/MSCHWERN/ExtUtils-MakeMaker-6.64/Changes, the support was added in version 6.06_01. But then there were various bugs discovered. I don't know since when exactly it's reliable.
TODO: Specify version constrain `>= 1.14' at perl(Test::Pod) (t/pod.t:4). TODO: Specify version constrain `>= 1.04' at perl(Test::Pod::Coverage) (t/pod-coverage.t:4).
Is that really necessary?
I mean, even EPEL 4 has recent enough versions...
In general, I'm not friend of implicit dependencies. Specifying exact dependencies makes the package safer and more portable. (Imagine someone could try the package on older or completely different distribution.)
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=901387
--- Comment #4 from Mathieu Bridon bochecha@fedoraproject.org --- (In reply to comment #3)
(In reply to comment #2)
(In reply to comment #1)
TODO: Replace PERL_INSTALL_ROOT variable with DESTDIR in %install section. Current ExtUtils::MakeMaker understands DESTDIR.
Out of curiosity, do you know what is the minimal version of ExtUtils::MakeMaker which started understanding DESTDIR? (to avoid closing the door to EPEL builds, in this package or in my future submissions)
According to ExtUtils::MakeMaker changelog http://cpansearch.perl.org/src/MSCHWERN/ExtUtils-MakeMaker-6.64/Changes, the support was added in version 6.06_01. But then there were various bugs discovered. I don't know since when exactly it's reliable.
Ok, so I'd rather keep PERL_INSTALL_ROOT for some time then.
TODO: Specify version constrain `>= 1.14' at perl(Test::Pod) (t/pod.t:4). TODO: Specify version constrain `>= 1.04' at perl(Test::Pod::Coverage) (t/pod-coverage.t:4).
Is that really necessary?
I mean, even EPEL 4 has recent enough versions...
In general, I'm not friend of implicit dependencies. Specifying exact dependencies makes the package safer and more portable. (Imagine someone could try the package on older or completely different distribution.)
Point taken. Fixed.
(In reply to comment #3)
(In reply to comment #2)
(In reply to comment #1)
TODO: You can replace %{__perl} macro with plain perl command.
Fixed.
Updated spec file is on the same address?
I hadn't pushed it, because you had approved the package and asked to fix before building for Fedora.
But since I commented here, I should have anyway. Here it is:
Spec URL: http://bochecha.fedorapeople.org/packages/perl-Text-Affixes-0.07-2.fc18.src.... SRPM URL: http://bochecha.fedorapeople.org/packages/perl-Text-Affixes.spec
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=901387
Mathieu Bridon bochecha@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-cvs?
--- Comment #5 from Mathieu Bridon bochecha@fedoraproject.org --- (In reply to comment #4)
(In reply to comment #3)
(In reply to comment #2)
(In reply to comment #1)
TODO: Replace PERL_INSTALL_ROOT variable with DESTDIR in %install section. Current ExtUtils::MakeMaker understands DESTDIR.
Out of curiosity, do you know what is the minimal version of ExtUtils::MakeMaker which started understanding DESTDIR? (to avoid closing the door to EPEL builds, in this package or in my future submissions)
According to ExtUtils::MakeMaker changelog http://cpansearch.perl.org/src/MSCHWERN/ExtUtils-MakeMaker-6.64/Changes, the support was added in version 6.06_01. But then there were various bugs discovered. I don't know since when exactly it's reliable.
Ok, so I'd rather keep PERL_INSTALL_ROOT for some time then.
Reading the changelog again, the last version which mentions a change related to DESTDIR is 6.25_08, released almost 8 years ago.
EL 6 (I couldn't even find the ExtUtils::MakeMaker for earlier EL releases) has version 6.55, so it seems this should be pretty safe by now. :)
So I changed that, here it is.
Spec URL: http://bochecha.fedorapeople.org/packages/perl-Text-Affixes-0.07-3.fc18.src.... SRPM URL: http://bochecha.fedorapeople.org/packages/perl-Text-Affixes.spec
I just realized I still haven't made the SCM request though...
New Package SCM Request ======================= Package Name: perl-Text-Affixes Short Description: Prefixes and suffixes analysis of text Owners: bochecha Branches: devel InitialCC: perl-sig
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=901387
--- Comment #6 from Jon Ciesla limburgher@gmail.com --- Git done (by process-git-requests).
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=901387
Mathieu Bridon bochecha@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution|--- |NEXTRELEASE Last Closed| |2013-01-24 23:41:35
--- Comment #7 from Mathieu Bridon bochecha@fedoraproject.org --- Thanks Petr for the review, and thank you Jon for the Git process.
Package build in Rawhide, closing.
package-review@lists.fedoraproject.org