[Bug 476346] Review Request: python-polybori - Framework for Boolean Rings
bugzilla at redhat.com
bugzilla at redhat.com
Tue Mar 24 20:07:50 UTC 2009
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=476346
--- Comment #14 from Conrad Meyer <konrad at tylerc.org> 2009-03-24 16:07:29 EDT ---
(In reply to comment #13)
> * %define -> %global
Done.
> * %debug_package
> Perhaps modifying "LDFLAGS_LINUX" in polybori/Makefile.in will
> solve this issue.
This doesn't fix it --- I also removed -s as a link flag from SConstruct, and
now it builds without stripping at link time.
> * License
> - Seems GPLv2+. Would you check this?
Ah. The only source I could find (sourceforge.net) just said GPL, so I assumed
GPLv3. I guess LICENSE says GPLv2+, good (and at least one source file says the
license information is in LICENSE).
> * Requires
> This means -devel subpackage should have "Requires: boost-devel"
> (here I am not saying about BuildRequires).
Good catch; fixed.
> -----------------------------------------------------------------
> # rpm -ql python-polybori-devel | xargs grep -h 'include ' | sort | uniq
> -----------------------------------------------------------------
> will show some useful information.
> ! And this output shows that polybori/CDDManager.h contains:
> -----------------------------------------------------------------
> 102 #define CDDManager_h_
> 103 #include "cacheopts.h"
> 104 // load basic definitions
> 105 #include "pbori_defs.h"
> -----------------------------------------------------------------
> Would you check if cacheopts.h can be really removed?
Well, it's an empty file and rpm complains about that. I will not remove it,
for now.
> - Also please check the dependency for main package.
> This may mean that python-polybori should have "Requires:
> python-imaging".
Done, thanks for spotting that.
> $ LANG=C ipbori
> /usr/bin/ipbori: line 66: ipython: command not found
> -----------------------------------------------------------------
> Perhaps "Requires: ipython" is needed.
Yep.
> * SourceURL
> - Please follow:
> https://fedoraproject.org/wiki/Packaging/SourceURL#Sourceforge.net
Thanks, I couldn't find that when I was originally packaging this. Fixed.
> * Linkage on installed system libraries
> - These two libraries undefined non-weak symbols.
> This cannot be allowed when these libraries also have in
> -devel subpackage named "libfoo.so" used for linkage against
> these libraries, because these symbols will cause linkage error.
What do I need to fix for this?
> * Documents
> - I prefer to include README as %doc of main package for this case.
> - ChangeLog seems useful for -devel subpackage.
Ok, added.
Update URLs:
http://konradm.fedorapeople.org/fedora/SPECS/python-polybori.spec
http://konradm.fedorapeople.org/fedora/SRPMS/python-polybori-0.5-4.fc10.src.rpm
--
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.
More information about the Fedora-package-review
mailing list