Bug 705798 - Review Request: perl-Data-Hexify - Perl extension for hexdumping arbitrary data
Summary: Review Request: perl-Data-Hexify - Perl extension for hexdumping arbitrary data
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: 705799
TreeView+ depends on / blocked
 
Reported: 2011-05-18 14:07 UTC by Richard W.M. Jones
Modified: 2011-06-21 17:25 UTC (History)
5 users (show)

Fixed In Version: perl-Data-Hexify-1.00-1.fc15
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2011-06-21 17:25:51 UTC
Type: ---
ppisar: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Richard W.M. Jones 2011-05-18 14:07:32 UTC
Spec URL: http://oirase.annexia.org/reviews/evtx/perl-Data-Hexify.spec
SRPM URL: http://oirase.annexia.org/reviews/evtx/perl-Data-Hexify-1.00-1.fc15.src.rpm
Description: Perl extension for hexdumping arbitrary data

Comment 1 Richard W.M. Jones 2011-05-18 14:08:34 UTC
Koji scratch build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=3078701

Comment 2 Richard W.M. Jones 2011-05-18 14:09:29 UTC
rpmlint output can all be ignored I think:

perl-Data-Hexify.src: W: spelling-error Summary(en_US) hexdumping -> hex dumping, hex-dumping, thumping
perl-Data-Hexify.src: W: spelling-error %description -l en_US hexdumping -> hex dumping, hex-dumping, thumping
perl-Data-Hexify.noarch: W: spelling-error Summary(en_US) hexdumping -> hex dumping, hex-dumping, thumping
perl-Data-Hexify.noarch: W: spelling-error %description -l en_US hexdumping -> hex dumping, hex-dumping, thumping
2 packages and 0 specfiles checked; 0 errors, 4 warnings.

Comment 3 Ralf Corsepius 2011-05-19 06:11:02 UTC
One remark:

perl(Test::More) is a build-time-only requirement. It is not required at run-time.

=> R: perl(Test::More) should be removed.

Comment 4 Richard W.M. Jones 2011-05-19 07:49:21 UTC
(In reply to comment #3)
> One remark:
> 
> perl(Test::More) is a build-time-only requirement. It is not required at
> run-time.
> 
> => R: perl(Test::More) should be removed.

Agreed.  I removed that line from the spec file, although
I didn't bother to rebuild the SRPM.

Comment 5 Petr Pisar 2011-06-09 12:02:55 UTC
Source tar ball is original. Ok.
Summary verified from lib/Data/Hexify.pm. Ok.
License verified from lib/Data/Hexify.pm. Ok.

TODO: You can remove BuildRoot: definition and all occurrences of it's deletion (as the whole %clean section) if you are not going to package this software for EPEL as they are not needed in Fedora anymore (they are done by rpmbuild automatically).

Package does not contain Perl C binding. noarch architecture is Ok.
Description is reasonable (maybe replacing `hexdumping' with some codified words would be better). Ok.

FIX: BuildRequire perl(Exporter) because of running test as this module can dual-live in the future (lib/Data/Hexify.pm:18, http://search.cpan.org/~ferreira/Exporter/).

TODO: Remove %defattr from %files section as it's done automatically by rpmbuild.

All tests pass. Ok.

$ rpmlint perl-Data-Hexify.spec ../SRPMS/perl-Data-Hexify-1.00-1.fc15.src.rpm ../RPMS/noarch/perl-Data-Hexify-1.00-1.fc15.noarch.rpm 
perl-Data-Hexify.src: W: spelling-error Summary(en_US) hexdumping -> hex dumping, hex-dumping, thumping
perl-Data-Hexify.src: W: spelling-error %description -l en_US hexdumping -> hex dumping, hex-dumping, thumping
perl-Data-Hexify.noarch: W: spelling-error Summary(en_US) hexdumping -> hex dumping, hex-dumping, thumping
perl-Data-Hexify.noarch: W: spelling-error %description -l en_US hexdumping -> hex dumping, hex-dumping, thumping
2 packages and 1 specfiles checked; 0 errors, 4 warnings.

I recommend to use the hyphen variation. rpmlint is OK.

$ rpm -q -lv -p ../RPMS/noarch/perl-Data-Hexify-1.00-1.fc15.noarch.rpm 
drwxr-xr-x    2 root    root                        0 čen  9 13:54 /usr/share/doc/perl-Data-Hexify-1.00
-rw-r--r--    1 root    root                      224 lis  5  2004 /usr/share/doc/perl-Data-Hexify-1.00/Changes
-rw-r--r--    1 root    root                     1627 čen 29  2004 /usr/share/doc/perl-Data-Hexify-1.00/README
-rw-r--r--    1 root    root                     3085 čen  9 13:54 /usr/share/man/man3/Data::Hexify.3pm.gz
drwxr-xr-x    2 root    root                        0 čen  9 13:54 /usr/share/perl5/vendor_perl/Data
-rw-r--r--    1 root    root                     8407 lis  5  2004 /usr/share/perl5/vendor_perl/Data/Hexify.pm
File permissions and layout is Ok.

$ rpm -q --requires -p ../RPMS/noarch/perl-Data-Hexify-1.00-1.fc15.noarch.rpm |sort |uniq -c
      1 perl(bytes)  
      1 perl(Carp)  
      1 perl(Exporter)  
      1 perl(:MODULE_COMPAT_5.12.3)  
      1 perl(strict)  
      1 perl(warnings)  
      1 perl >= 0:5.006
      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 Ok.

$ rpm -q --provides -p ../RPMS/noarch/perl-Data-Hexify-1.00-1.fc15.noarch.rpm |sort |uniq -c
      1 perl(Data::Hexify) = 1.00
      1 perl-Data-Hexify = 1.00-1.fc15
Binary provides Ok.

$ resolvedeps rawhide ../RPMS/noarch/perl-Data-Hexify-1.00-1.fc15.noarch.rpm 
Binary dependencies resolvable. Ok.

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

Otherwise package is in line with Fedora and perl packaging guidelines.


Please correct all `FIX' prefixed issues, consider fixing `TODO' issues, and provide new spec file.

Resolution: Package NOT approved.

Comment 6 Richard W.M. Jones 2011-06-09 12:16:08 UTC
Updated package (I rebuilt it but didn't bump the release):

http://oirase.annexia.org/reviews/evtx/perl-Data-Hexify.spec
http://oirase.annexia.org/reviews/evtx/perl-Data-Hexify-1.00-1.fc15.src.rpm

This should address all of the concerns there.

rpmlint is still complaining that "hexdump" isn't a word, but
I think it's a perfectly cromulent word.

Comment 7 Petr Pisar 2011-06-09 12:52:07 UTC
Spec file changes:

--- perl-Data-Hexify.spec	2011-05-19 09:49:02.000000000 +0200
+++ perl-Data-Hexify.spec.1	2011-06-09 14:14:47.000000000 +0200
@@ -1,20 +1,20 @@
 Name:           perl-Data-Hexify
 Version:        1.00
 Release:        1%{?dist}
-Summary:        Perl extension for hexdumping arbitrary data
+Summary:        Perl extension to hexdump arbitrary data
 License:        GPL+ or Artistic
 Group:          Development/Libraries
 URL:            http://search.cpan.org/dist/Data-Hexify/
 Source0:        http://www.cpan.org/authors/id/J/JV/JV/Data-Hexify-%{version}.tar.gz
-BuildRoot:      %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
 BuildArch:      noarch
 BuildRequires:  perl(ExtUtils::MakeMaker)
 BuildRequires:  perl(Test::More)
+BuildRequires:  perl(Exporter)
 Requires:       perl(:MODULE_COMPAT_%(eval "`%{__perl} -V:version`"; echo $version))
 
 
 %description
-Perl extension for hexdumping arbitrary data.
+Perl extension to hexdump arbitrary data.
 
 
 %prep
@@ -46,7 +46,6 @@
 
 
 %files
-%defattr(-,root,root,-)
 %doc Changes README
 %{perl_vendorlib}/*
 %{_mandir}/man3/*


> TODO: You can remove BuildRoot: definition and all occurrences of it's
> deletion (as the whole %clean section) if you are not going to package this
> software for EPEL as they are not needed in Fedora anymore (they are done by
> rpmbuild automatically).
-BuildRoot:      %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
Ok.

TODO: I meant to remove `rm -rf $RPM_BUILD_ROOT' commands either and the %clean section itself as it became empty.


> FIX: BuildRequire perl(Exporter) because of running test as this module can
> dual-live in the future (lib/Data/Hexify.pm:18,
> http://search.cpan.org/~ferreira/Exporter/).
+BuildRequires:  perl(Exporter)
Ok.

> TODO: Remove %defattr from %files section as it's done automatically by
> rpmbuild.
-%defattr(-,root,root,-)
Ok.

$ rpmlint perl-Data-Hexify.spec ../SRPMS/perl-Data-Hexify-1.00-1.fc15.src.rpm ../RPMS/noarch/perl-Data-Hexify-1.00-1.fc15.noarch.rpm 
perl-Data-Hexify.src: W: spelling-error Summary(en_US) hexdump -> hex dump, hex-dump, headlamp
perl-Data-Hexify.src: W: spelling-error %description -l en_US hexdump -> hex dump, hex-dump, headlamp
perl-Data-Hexify.noarch: W: spelling-error Summary(en_US) hexdump -> hex dump, hex-dump, headlamp
perl-Data-Hexify.noarch: W: spelling-error %description -l en_US hexdump -> hex dump, hex-dump, headlamp
2 packages and 1 specfiles checked; 0 errors, 4 warnings.

I don't think changing noun to verb with the same stem is such an improvement.

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


Please consider fixing the `TODO' issues before building the package.

Resolution: Package APPROVED.

Comment 8 Richard W.M. Jones 2011-06-09 13:17:41 UTC
I've removed the two rm -rf commands and the %clean section.

Comment 9 Richard W.M. Jones 2011-06-09 13:25:59 UTC
Thanks for the review!  Here is the git request.

New Package SCM Request
=======================
Package Name: perl-Data-Hexify
Short Description: Perl extension to hexdump arbitrary data
Owners: rjones
Branches: f15
InitialCC:

Comment 10 Petr Pisar 2011-06-09 13:41:37 UTC
Please add `perl-sig' to InitalCC of SCM Request structure. All Perl packages should do so.

Comment 11 Richard W.M. Jones 2011-06-09 13:51:22 UTC
New Package SCM Request
=======================
Package Name: perl-Data-Hexify
Short Description: Perl extension to hexdump arbitrary data
Owners: rjones
Branches: f15
InitialCC: perl-sig

Comment 12 Gwyn Ciesla 2011-06-09 13:56:20 UTC
Git done (by process-git-requests).

Comment 13 Richard W.M. Jones 2011-06-09 14:12:18 UTC
Thanks everyone.

I've pushed this and built it in Rawhide and F15:

http://koji.fedoraproject.org/koji/taskinfo?taskID=3121708
http://koji.fedoraproject.org/koji/taskinfo?taskID=3121710

Comment 14 Fedora Update System 2011-06-09 14:13:24 UTC
perl-Data-Hexify-1.00-1.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/perl-Data-Hexify-1.00-1.fc15

Comment 15 Fedora Update System 2011-06-11 04:23:42 UTC
perl-Data-Hexify-1.00-1.fc15 has been pushed to the Fedora 15 testing repository.

Comment 16 Fedora Update System 2011-06-21 17:25:44 UTC
perl-Data-Hexify-1.00-1.fc15 has been pushed to the Fedora 15 stable repository.


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