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=751119 --- Comment #6 from Petr Šabata <psabata@xxxxxxxxxx> 2012-02-20 12:57:33 EST --- I guess I could sponsor you. I'd like to see some more packages from you first, though... Formal review: -------------- FIX: The package doesn't build (tested in Rawhide) due to missing build-time dependencies. The following should be added: perl(constant), perl(Sub::Exporter) FIX: I don't see a reason for those custom macros -- could you explain them? Anyway, use %global instead of %define to comply with the Guidelines. FIX: Remove './Build test' from %build and move it to a (not currently present) %check section FIX: Remove gcc from build-time dependencies. This is always present. FIX: Remove all those useless explicit Provides. TODO: Improve the %description; this is just a copy of Summary and doesn't really say much about the package. TODO: Unless you plan this for EPEL, remove the %clean section. TODO: Unless you plan this for EPEL5 or older, drop the buildroot removal from %install. If you do, add %defattr to %files and specify the Buildroot tag. TODO: Don't list manpages in %doc. They're marked as documentation automatically. Also substitute the gzip extension with a glob; the compression might change in the future. Be ready. FIX: Don't use paths in %doc; simple '%doc Changes README' would do the right thing (TM). FIX: Simply put: Use Module::Build as other packages do. You don't need anything extra. E.g.: %build perl Build.PL installdirs=vendor ./Build %install ./Build install destdir=%{buildroot} create_packlist=0 find %{buildroot} -depth -type d -exec rmdir {} 2>/dev/null \; %{_fixperms} %{buildroot}/* %check ./Build test TIP: Simplify the %files list by using globs. FIX: Your Version is missing the %dist-tag. Add it. TIP: What's the reason for the patch? Explain it in a comment. -- Not approving yet. The FIX items are blockers, TODOs are 'just' highly recommended. -- 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