[Bug 625855] Review Request: perl-EBook-EPUB - Perl module for generating EPUB documents

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=625855

--- Comment #1 from Steve Traylen <steve.traylen@xxxxxxx> 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

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review


[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]