[Bug 751119] Review Request: perl-Judy - Perl wrapper for Judy

[bugzilla at redhat.com]

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 at redhat.com> 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.