Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=886300
--- Comment #5 from François Cami <fdc(a)fcami.net> ---
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