Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=886300
--- Comment #4 from Pavel Raiskup <praiskup(a)redhat.com> ---
Hi François, thanks for packaging! Here are some comments:
* Note that: Explicit dependency on perl-devel is not allowed (consider
Module::Build). For more info [1].
* There should be also BR MODULE_COMPAT_* dependency [1]:
perl(:MODULE_COMPAT_%(eval "`%{__perl} -V:version`"; echo $version))
* Weird thing is, that the sino does not require libsino, is it statically
linked. Is that necessary?
* Explicit library dependency shouldn't be needed [2]. If yes, the %{_isa}
should be given & comment added.
* No subpackage -> subpackage dependancy is given (except the lib one). Is
that
correct?
* I'm not sure about:
%post & %postun -n perl-sinoAPI -p /sbin/ldconfig
I'm not a perl guru but as this subpackage does not install libraries
into ldconfig path, and thus it should not be needed?
* imo we don't need %posttrans * scriptlets also
* Your hacks for so library may look ugly, would it be possible to be
solved upstream? Just a note now for this, the 'mv -f lib/libsino.so
lib/libsino.so.3' command seems to redundant there.
* License seems to be s/GPLv3/GPLv3+/ and license of malloc.c seems to be not
GPLv3+ (btw. this file seems to be unused, but it is quite good - seems to be
very similar to what tcsh has (tc.alloc.c) and it causes weird kernel
problems #443643).
* Manual page for sinodisp is not found. I could be considered like small
problem, but I can't find any documentation on this! No --help, no manual
page, also 'rpm -qd sino-cgi' is silent. No documentation found.
Nits:
* I would comment the patches a little. Downstream/upstream, etc.
* consider adding -p to install command (preserve timestamps)
* IIRC, defattr is not needed even in EPEL5, version in RHEL5 is 4.4.2.3.
Thanks for working on this,
Pavel
[1]
https://fedoraproject.org/wiki/Packaging:Perl
[2]
https://fedoraproject.org/wiki/Packaging:Guidelines#Explicit_Requires
--
You are receiving this mail because:
You are on the CC list for the bug.
Unsubscribe from this bug
https://bugzilla.redhat.com/token.cgi?t=X2ca1sNhJm&a=cc_unsubscribe