[Bug 539472] New: Review Request: libmemcache - C API to memcache

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

Summary: Review Request: libmemcache - C API to memcache

https://bugzilla.redhat.com/show_bug.cgi?id=539472

Summary: Review Request: libmemcache - C API to memcache
           Product: Fedora
           Version: rawhide
          Platform: All
        OS/Version: Linux
            Status: NEW
          Severity: medium
          Priority: medium
         Component: Package Review
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]
         QAContact: [e-mail]
                CC: [e-mail], [e-mail]
   Estimated Hours: 0.0
    Classification: Fedora

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.rp...
Description: libmemcache is a C API library to the memcached server
(http://danga.com/memcached).

[Bug 539472] Review Request: libmemcache - C API to memcache

[Red Hat Bugzilla]

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=539472

Jeroen van Meeuwen  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |539469

[Bug 539472] Review Request: libmemcache - C API to memcached

[Red Hat Bugzilla]

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=539472

Jeroen van Meeuwen  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|Review Request: libmemcache |Review Request: libmemcache
                   |- C API to memcache         |- C API to memcached

[Bug 539472] Review Request: libmemcache - C API to memcached

[Red Hat Bugzilla]

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=539472

--- Comment #1 from Jeroen van Meeuwen   2009-11-21 10:54:59 EDT ---
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.rp...

[Bug 539472] Review Request: libmemcache - C API to memcached

[Red Hat Bugzilla]

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=539472

--- Comment #2 from Michael Schwendt   2009-11-23 08:34:04 EDT ---
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.

[Bug 539472] Review Request: libmemcache - C API to memcached

[Red Hat Bugzilla]

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=539472

Remi Collet  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #3 from Remi Collet   2009-12-30 12:53:39 EDT ---
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.

+

[Bug 539472] Review Request: libmemcache - C API to memcached

[Red Hat Bugzilla]

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=539472

Jeroen van Meeuwen  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |CLOSED
         Resolution|                            |DEFERRED

--- Comment #4 from Jeroen van Meeuwen   2009-12-31 08:23:28 EDT ---
OpenSRF has been ported to use libmemcached instead. Closing deferred.