Bug 625855 - Review Request: perl-EBook-EPUB - Perl module for generating EPUB documents
Review Request: perl-EBook-EPUB - Perl module for generating EPUB documents
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Steve Traylen
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2010-08-20 11:47 EDT by Emmanuel Seyman
Modified: 2010-09-17 14:27 EDT (History)
3 users (show)

See Also:
Fixed In Version: perl-EBook-EPUB-0.5-2.fc13
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2010-09-15 23:45:41 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
steve.traylen: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Emmanuel Seyman 2010-08-20 11:47:31 EDT
Spec URL: http://people.parinux.org/~seyman/fedora/perl-EBook-EPUB/perl-EBook-EPUB.spec
SRPM URL: http://people.parinux.org/~seyman/fedora/perl-EBook-EPUB/perl-EBook-EPUB-0.5-1.fc13.src.rpm
Description:
This module permits creating EPUB documents.
Comment 1 Steve Traylen 2010-08-25 04:26:33 EDT
Review of perl(EBook::EPUB) , 25th August 2010.

mock in f15 fails, missing build requires.

PERL_DL_NONLAZY=1 /usr/bin/perl "-MExtUtils::Command::MM" "-e" "test_harness(0, 'blib/lib', 'blib/arch')" t/*.t
#   Failed test 'use EBook::EPUB;'
#   at t/00-load.t line 6.
#     Tried to use 'EBook::EPUB'.
#     Error:  Can't locate version.pm in @INC (@INC contains: /builddir/build/BUILD/EBook-EPUB-0.5/blib/lib /builddir/build/BUILD/EBook-EPUB-0.5/blib/arch /usr/local/lib64/perl5 /usr/local/share/perl5 /usr/lib64/perl5 /usr/share/perl5 /usr/lib64/perl5 /usr/share/perl5 /usr/local/lib64/perl5/site_perl/5.10.0/x86_64-linux-thread-multi /usr/local/lib/perl5/site_perl/5.10.0 /usr/lib64/perl5/vendor_perl/5.10.0/x86_64-linux-thread-multi /usr/lib/perl5/vendor_perl /usr/lib/perl5/site_perl) at /builddir/build/BUILD/EBook-EPUB-0.5/blib/lib/EBook/EPUB.pm line 27.
# BEGIN failed--compilation aborted at /builddir/build/BUILD/EBook-EPUB-0.5/blib/lib/EBook/EPUB.pm line 27.


- Package meets naming and packaging guidelines
Yes CPAN module called EBook::EPUB
- Spec file matches base package name.
Yes
- Spec has consistant macro usage.
Yes.
- Meets Packaging Guidelines.
Almost  but comments below concening ignore.txt 
- License
BSD.
- License field in spec matches
Yes, the README and the headers of the files make this clearly
BSD but they do say to go and read the LICESNE file.
- License file included in package
No it's not but it is referenced in the README.

This module is free software; you can redistribute it and/or modify it 
under the terms of the BSD license. See the LICENSE file included with 
this distribution.

So I think it is important in this case that upstream can be contacted
to try and add a LICENSE files.


- Spec in American English
- Spec is legible.
- Sources match upstream md5sum:
$ md5sum EBook-EPUB-0.5.tar.gz ../SOURCES/EBook-EPUB-0.5.tar.gz 
ed6a49cba2a173db3c479b229aec0cc2  EBook-EPUB-0.5.tar.gz
ed6a49cba2a173db3c479b229aec0cc2  ../SOURCES/EBook-EPUB-0.5.tar.gz

- Package needs ExcludeArch
No it does not
- BuildRequires correct
Mock okay.
- Spec handles locales/find_lang
None present.
- Package is relocatable and has a reason to be.
Not relocatable.
- Package has %defattr and permissions on files is good.
It does
- Package has a correct %clean section.
It does.
- Package has correct buildroot
%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
- Package is code or permissible content.
- Doc subpackage needed/used.
- Packages %doc files don't affect runtime.

- Headers/static libs in -devel subpackage.
- Spec has needed ldconfig in post and postun
- .pc files in -devel subpackage/requires pkgconfig
- .so files in -devel subpackage.
- -devel package Requires: %{name} = %{version}-%{release}
- .la files are removed.

- Package is a GUI app and has a .desktop file

- Package compiles and builds on at least one arch.
- Package has no duplicate files in %files.
- Package doesn't own any directories other packages own.
- Package owns all the directories it creates.
- No rpmlint output.
- final provides and requires are sane:



Issues:

1. I think there is no reason to include ignore.txt , it adds nothing
   to the package.
2. Can you contact upstream to request a LICENSE file.
3. I have been asked previously to change:


make pure_install PERL_INSTALL_ROOT=$RPM_BUILD_ROOT

to

make pure_install DESTDIR=$RPM_BUILD_ROOT

and is something I always do.


4. The fail in mock of course.

Steve
Comment 2 Emmanuel Seyman 2010-08-26 19:15:08 EDT
(In reply to comment #1)
>
> 1. I think there is no reason to include ignore.txt , it adds nothing
>    to the package.

You're right, done.

> 2. Can you contact upstream to request a LICENSE file.

Done. http://rt.cpan.org/Ticket/Display.html?id=60759

> 3. I have been asked previously to change:
> 
> make pure_install PERL_INSTALL_ROOT=$RPM_BUILD_ROOT
> to
> make pure_install DESTDIR=$RPM_BUILD_ROOT
> 
> and is something I always do.

Done.

> 4. The fail in mock of course.

Fixed by adding perl(version) in the BuildRequires

Spec URL:
http://people.parinux.org/~seyman/fedora/perl-EBook-EPUB/perl-EBook-EPUB.spec
SRPM URL:
http://people.parinux.org/~seyman/fedora/perl-EBook-EPUB/perl-EBook-EPUB-0.5-2.fc13.src.rpm
Comment 3 Steve Traylen 2010-08-27 05:03:53 EDT
You man want to add a 

%{?perl_default_subpackage_tests}

after the preamble to generate the -test package. Up to you.

+ builds okay in mock.
+ rpmlint output completely clean.

APPROVED.
Comment 4 Emmanuel Seyman 2010-08-27 09:41:09 EDT
(In reply to comment #3)
>
> You man want to add a 
> %{?perl_default_subpackage_tests}
> after the preamble to generate the -test package. Up to you.

Good idea. Added.

> APPROVED.

Thanks for the review, Steve.

New Package SCM Request
=======================
Package Name: perl-EBook-EPUB
Short Description: Perl module for generating EPUB documents
Owners: eseyman
Branches: f12 f13 f14
InitialCC: perl-sig
Comment 5 Kevin Fenzi 2010-08-29 15:01:30 EDT
Git done (by process-git-requests).
Comment 6 Fedora Update System 2010-08-30 16:32:12 EDT
perl-EBook-EPUB-0.5-2.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/perl-EBook-EPUB-0.5-2.fc14
Comment 7 Fedora Update System 2010-08-30 16:32:17 EDT
perl-EBook-EPUB-0.5-2.fc13 has been submitted as an update for Fedora 13.
https://admin.fedoraproject.org/updates/perl-EBook-EPUB-0.5-2.fc13
Comment 8 Fedora Update System 2010-08-30 16:32:22 EDT
perl-EBook-EPUB-0.5-2.fc12 has been submitted as an update for Fedora 12.
https://admin.fedoraproject.org/updates/perl-EBook-EPUB-0.5-2.fc12
Comment 9 Fedora Update System 2010-08-31 02:32:19 EDT
perl-EBook-EPUB-0.5-2.fc12 has been pushed to the Fedora 12 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update perl-EBook-EPUB'.  You can provide feedback for this update here: https://admin.fedoraproject.org/updates/perl-EBook-EPUB-0.5-2.fc12
Comment 10 Fedora Update System 2010-09-15 23:45:37 EDT
perl-EBook-EPUB-0.5-2.fc14 has been pushed to the Fedora 14 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 11 Fedora Update System 2010-09-17 14:24:18 EDT
perl-EBook-EPUB-0.5-2.fc12 has been pushed to the Fedora 12 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 12 Fedora Update System 2010-09-17 14:27:01 EDT
perl-EBook-EPUB-0.5-2.fc13 has been pushed to the Fedora 13 stable repository.  If problems still persist, please make note of it in this bug report.

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