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=476600 --- Comment #2 from Conrad Meyer <konrad@xxxxxxxxxx> 2009-10-27 21:25:56 EDT --- (In reply to comment #1) > Mock build fails. Against what? Rawhide? Fedora 11? Fedora 10? As you noted lower down in your initial review, there's a Python 2.6 incompatibility in this (Fedora 9!) package. I will update to the newer version later today, which should fix this issue. > Why group Development/Languages when it isn't a language? I would assume > Development/Libraries would fit better. Many Python libraries are Development/Languages (the default in the rpmdev-spec template for python packages). In my personal opinion, RPM Group tags are totally useless at this point, and accordingly I don't have a strong opinion on this. > URL should point to a version-independent site, I guess that it should be > http://pypi.python.org/pypi/ZODB3/ instead. ACK. > Unused BuildRequires should not just be commented out. Remove them if they are > invalid. ACK. > The packages for python-zope-proxy and python-zdaemon are not available in > rawhide, so this package can't be properly reviewed yet. What you mean to say is "fully", not "properly". This package can't be approved yet, but it can certainly go through most of the review process. > Source should not be modified in %prep; it is a kind of build process and > belongs in %build. Has this problem with invalid hash-bangs been filed and > discussed upstream? This is complete fiction. %prep is the place for modifying source -- patches, sed scriptlets, etc go here. Nothing has been filed upstream, because it's fairly minor change IMO. I would be glad to contact upstream about it if that's a blocker. > I am not familiar with CFLAGS combined with setup.py. I assume that the right > flags from python-devel is used automatically. I think the python-numeric > package is similar to this one in many ways, and even though it does many > things wrong it can perhaps be used for reference: It do not list CFLAGS. Can > you reference any documentation or prior art from other packages? file:///etc/rpmdevtools/spectemplate-python.spec (if you have the rpmdevtools package installed). > The mv code in %install seems overly complex and clever. > Couldn't it be written as > mv > $RPM_BUILD_ROOT%{_bindir}/{fsdump,fsoids,fsrefs,fstail,mkzeoinst,repozo,runzeo,zeoctl,zeopack,zeopasswd} > \ > $RPM_BUILD_ROOT%{_libexecdir}/%{name}/ > or just > mv $RPM_BUILD_ROOT%{_bindir}/* $RPM_BUILD_ROOT%{_libexecdir}/%{name}/ > ? These are all identical and don't win us anything. > A %check section should either be there or not - not just contain a disabled > checkout without further comments. I forget why I have it disabled, but probably the tests either fail, or python-zope-testing wasn't yet in Fedora when this spec file was originally written. I think leaving a way to remember how to run the tests is valuable even if they are disabled currently. > When building I notice an error which should be fixed: > byte-compiling > /home/mk/rpmbuild/BUILDROOT/python-ZODB3-3.9.0-0.1.a7.fc12.i386/usr/lib/python2.6/site-packages/ZEO/scripts/zeoserverlog.py > to zeoserverlog.pyc > SyntaxError: ('invalid syntax', > ('/usr/lib/python2.6/site-packages/ZEO/scripts/zeoserverlog.py', 374, 6, ' > as = []\n')) Looks like a new Python 2.6 keyword; this is probably fixed in a newer version (such as 3.9.3, which I'll update the spec/srpm to later today). > rpmlint output: > python-ZODB3.i686: W: devel-file-in-non-devel-package > /usr/lib/python2.6/site-packages/persistent/*.[ch] > python-ZODB3.i686: W: devel-file-in-non-devel-package > /usr/lib/python2.6/site-packages/BTrees/*.[ch] > python-ZODB3.i686: W: devel-file-in-non-devel-package > /usr/include/python2.6/ZODB3/*.h > 3 packages and 1 specfiles checked; 0 errors, 34 warnings. > The include files should perhaps go to a -devel package, and I guess the rest > of the files shouldn't be there at all. > /usr/include/python2.6/ZODB3 should be in a package which requires python-devel > which owns /usr/include/python2.6/ ACK. > It seems like this package contains 4 quite independent python modules. > Consider putting them in independent packages: python-BTrees python-persistent > python-ZEO python-ZODB I don't see any reason to; AFAIK they come from one upstream and are used only by themselves. > Hmm ... this brief review ends up being longer than the spec. IMHO there are > too many issues here. The packager should have worked more with the package > before filing a review request. Off-topic and rude -- not to mention, plenty of the assertions in this "brief review" were incorrect. -- 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. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review