Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=886300 --- Comment #5 from François Cami <fdc@xxxxxxxxx> --- Hi Pavel, Thank you very much for taking this review! * dependency on perl-devel => fixed. * MODULE_COMPAT_* dependency => fixed. * sino/sinomake are statically linked => this is how upstream builds. I plan to switch these binaries to dynamic linking at some point. * Explicit library dependency shouldn't be needed [2]. If yes, the %{_isa} should be given & comment added. => I am not sure of the line where this should be changed :/ * No subpackage -> subpackage dependancy is given (except the lib one). Is that correct? => I think so. * %post, %postun, %posttrans -n perl-sinoAPI -p /sbin/ldconfig => removed. * Your hacks for so library may look ugly, would it be possible to be solved upstream? => I wish, but I am pretty sure upstream needs to make sure nothing is broken on other UNIX like Solaris, and I do not have a Solaris test box. The alternative is to ship the static library and I would rather not do that... (and yes, the hack is ugly). * 'mv -f lib/libsino.so lib/libsino.so.3' redundant => I needed to move it out of the way, I switched to 'rm -f'. * license of malloc.c seems to be not GPLv3+ and this file seems to be unused => Good catch, added rm -f at the end of prep. * Manual page for sinodisp is not found (...) I can't find any documentation on this! => Sorry, I should have fixed it earlier. The documentation is at the beginning of sinodisp.c, so I now extract that and create a sinodisp.txt file which is installed by the sino-cgi package as documentation. I moved the sinodisp binary to /var/www/cgi-bin because that's where it really belongs. Nits: * I would comment the patches a little. Downstream/upstream, etc. => for now nothing's been upstreamed, but I noted what could, and could not be upstreamed, in the new spec. * consider adding -p to install command (preserve timestamps) => done. * IIRC, defattr is not needed even in EPEL5, version in RHEL5 is 4.4.2.3. => removed. thanks. Thank you! Spec URL: http://fcami.fedorapeople.org/srpms/sino.spec SRPM URL: http://fcami.fedorapeople.org/srpms/sino-3.1.21-3.fc18.src.rpm -- 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=G1ahyUCapi&a=cc_unsubscribe _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review