Spec URL: http://ruben.fedorapeople.org/python-zfec.spec SRPM URL: http://ruben.fedorapeople.org/python-zfec-1.4.5-1.fc11.src.rpm Description: Fast, portable, programmable erasure coding a.k.a. "forward error correction": the generation of redundant blocks of information such that if some blocks are lost then the original data can be recovered from the remaining blocks. This is one of the needed dependencies for the Tahoe LAFS distributed filesystem.
Review: Good: - name ok - BR ok - %build ok - %clean there - %doc ok - no *.la - no static libs - permissions ok - rpmlint clean Needswork: - %install: why don't you need the cmdtools? Make at least a comment in the spec. - %check section would be great, then you'd found out the next issue: - missing Requires: pyutil which is not yet in fedora... Also see the file zfec/filefec.py, there is also an import pyutil. I didn't test the programm, if it works, but it doesn't look like. Did you try it, or are you just packaging it as dependendy? Or did you remove the cmdtools because of this missing dependency? Better package that first, and enable all feature it has... - Be a bit more explicit in %files: %{python_sitearch}/%{name} and %{python_sitearch}/%{name}*.egg-info would be better. - Query upstream to add license headers. - Are the patches upstream
Hi Thomas, thanks for the review! Upstream released 1.4.6, which includes a few fixes for the issues you mentioned. I asked them to push the new version to pypi, when that's done I'll create a new version. Should be ready in a few days.
> - %install: why don't you need the cmdtools? I've added them. > - %check section would be great, then you'd found out the next issue: > - missing Requires: pyutil which is not yet in fedora... I've added the %check section, and packaged up pyutil (#560457) > - Be a bit more explicit in %files: > %{python_sitearch}/%{name} and %{python_sitearch}/%{name}*.egg-info would be > better. Ok, fixed. > - Query upstream to add license headers. Why, the license is included > - Are the patches upstream There's one patch left which removes darcsver, which isn't needed. Upstream uses it quite a lot. New SPEC: http://ruben.fedorapeople.org/python-zfec.spec New SRPM: http://ruben.fedorapeople.org/python-zfec-1.4.6-1.fc13.src.rpm
- sources matches upstream: 542957786c0b5de3ff76094349d0cf0d Only SHOULDS left: (In reply to comment #3) > > - Query upstream to add license headers. > Why, the license is included I remember, it's negative to not have headers, but can't find it in the guidelines. Only in COPYING.GPL it's just suggested in 'How to Apply These Terms to Your New Programs' too. Because you seem to be in contact with upstream, you could ask them to add the headers. But this is a should anyway... ;) - Deleting the tests would be good: Then there would be installed just what is needed. Here it doesn't matter much, because the tests are small... ############################################################## I'd approve this now, but I want to wait for the dependencies.
Hi Thomas, Ok, I'll delete the tests, and I've asked upstream to add license headers. Would you mind taking a look at pyutil and python-zbase32 as well?
FYI I approved pyutil and python-zbase.
s/python-zbase/python-zbase32/
(In reply to comment #6) > FYI I approved pyutil and python-zbase. Thank you. ######################################### APPROVED
New Package CVS Request ======================= Package Name: python-zfec Short Description: A fast erasure codec with python bindings Owners: ruben Branches: F-12
CVS done (by process-cvs-requests.py). Also added an F-13 branch.