Re: RFS: hiredis - Minimalistic C client library for Redis

[Andriy Senkovych <jolly_roger@itblog.org.ua>]

Hello, Alessandro

Thank you for your work on packaging this library. I'm not a DD so I
can't help with upload. However I use this library myself so I did
some work on packaging too. Hope this little review will help you to
get the package into Debian.

First of all, the package is FTBFS. The package needs to have redis
server (currently >= 1.2.0) installed to run the tests. There is no
redis-server in the Build-Depends.

In all other aspects the package appears to be pretty clean, so I
believe it's fine.

There are some other useful files you may want to include in your
package. These are pkgconfig file and CMake build configuration file.
I attached them to this mail. You might want to generate the pkgconfig
file as well or maybe propose both to the upstream.

Thanks again,
-- 
Andriy Senkovych

# - Try to find the Lib library
# Once done this will define
#
#  LIBHIREDIS_FOUND - System has Hiredis
#  LIBHIREDIS_INCLUDE_DIR - The Hiredis include directory
#  LIBHIREDIS_LIBRARIES - The libraries needed to use Hiredis
#  LIBHIREDIS_DEFINITIONS - Compiler switches required for using Hiredis


# use pkg-config to get the directories and then use these values
# in the FIND_PATH() and FIND_LIBRARY() calls
FIND_PACKAGE(PkgConfig)
PKG_SEARCH_MODULE(PC_LIBHIREDIS REQUIRED libhiredis)

SET(LIBHIREDIS_DEFINITIONS ${PC_LIBHIREDIS_CFLAGS_OTHER})

FIND_PATH(LIBHIREDIS_INCLUDE_DIR NAMES hiredis/hiredis.h
   HINTS
   ${PC_LIBHIREDIS_INCLUDEDIR}
   ${PC_LIBHIREDIS_INCLUDE_DIRS}
   PATH_SUFFIXES libhiredis
   )

FIND_LIBRARY(LIBHIREDIS_LIBRARIES NAMES hiredis
   HINTS
   ${PC_LIBHIREDIS_LIBDIR}
   ${PC_LIBHIREDIS_LIBRARY_DIRS}
   )


INCLUDE(FindPackageHandleStandardArgs)
FIND_PACKAGE_HANDLE_STANDARD_ARGS(LibHiredis DEFAULT_MSG LIBHIREDIS_LIBRARIES LIBHIREDIS_INCLUDE_DIR)

MARK_AS_ADVANCED(LIBHIREDIS_INCLUDE_DIR LIBHIREDIS_LIBRARIES)

Attachment: libhiredis.pc
Description: Binary data