Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=886300 --- Comment #4 from Pavel Raiskup <praiskup@xxxxxxxxxx> --- 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 _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review