Bug 539472

Summary:	Review Request: libmemcache - C API to memcached
Product:	[Fedora] Fedora	Reporter:	Jeroen van Meeuwen <vanmeeuwen+fedora>
Component:	Package Review	Assignee:	Nobody's working on this, feel free to take it <nobody>
Status:	CLOSED DEFERRED	QA Contact:	Fedora Extras Quality Assurance <extras-qa>
Severity:	medium	Docs Contact:
Priority:	medium
Version:	rawhide	CC:	fedora, fedora-package-review, notting
Target Milestone:	---
Target Release:	---
Hardware:	All
OS:	Linux
Whiteboard:
Fixed In Version:		Doc Type:	Bug Fix
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2009-12-31 13:23:28 UTC	Type:	---
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Embargoed:
Bug Depends On:
Bug Blocks:	539469

Description Jeroen van Meeuwen 2009-11-20 09:45:05 UTC

Spec URL: http://www.kanarip.com/custom/SPECS/libmemcache.spec
SRPM URL: http://www.kanarip.com/custom/f13/SRPMS/libmemcache-1.4.0-0.1rc2.fc13.src.rpm
Description: libmemcache is a C API library to the memcached server
(http://danga.com/memcached).

Comment 1 Jeroen van Meeuwen 2009-11-21 15:54:59 UTC

New SPEC: http://www.kanarip.com/custom/SPECS/libmemcache.spec
New SRPM: http://www.kanarip.com/custom/f13/SRPMS/libmemcache-1.4.0-0.2rc2.fc13.src.rpm

Comment 2 Michael Schwendt 2009-11-23 13:34:04 UTC

No full review, just some observations:


> rm -rf test/unit
>
> %{__rm} -rf %{buildroot}

Either use %{__rm} or "rm" consistently, but mixing them only raises doubts about whether using the macro %{__rm} is needed at all?


> %build
> rm -rf test/unit
> sed -i -e 's/unit//g' test/Makefile.am
> sed -i -e 's/test\/unit\/Makefile//g' configure.ac

That's a good example of stuff you ought to add comments to in the spec file. Not only to answer the "Why?" question, but also to confirm what this is supposed to achieve and whether the first sed translation might not kill anything unexpectedly with a future version upgrade.


> %files
> %defattr (-,root,root,-)
> %doc COPYING INSTALL ChangeLog
> ...
> 
> %files devel
> %defattr (-,root,root,-)
> %doc COPYING INSTALL ChangeLog
> ...

Is it really necessary to duplicate %doc files like that? Especially with Fedora, the -devel package requires the base package anyway.


> %{_includedir}/memcache*

'*' as in "many/any"?  Or as in "I don't care whether any version upgrade might move the API headers from to a location that's different from previous releases?


> %{_libdir}/%{name}.so.*

Macros, in particular %{name}, are overrated.  If you wanted to simply rename this package from "libmemcache" to "libmemcache1" or "compat-libmemcache1", you would need to touch the %files section, too. So, %{_libdir}/libmemcache.so.*  would be much more readable.

Comment 3 Remi Collet 2009-12-30 17:53:39 UTC

Do you really need this library ? it's seems unmaintained (lastest version is from 2006)

Your Source URL give a 404 and point to http://download.tangent.org/ which host "libmemcached" already available in repository.

+

Comment 4 Jeroen van Meeuwen 2009-12-31 13:23:28 UTC

OpenSRF has been ported to use libmemcached instead. Closing deferred.