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(a)tylerc.org> 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.