Code review comment for lp:~yolanda.robla/ubuntu/trusty/memcached/add_distribution

Revision history for this message
Barry Warsaw (barry) wrote :

This patch looks pretty good.

I noticed you used append_stat instead of APPEND_STAT (in memcached.c). It appears this is necessary because APPEND_STAT is a macro that only accepts 3 arguments (memcached.h).

I removed the extra newline from d/rules.

Some of the quilt patches had to be refreshed in order to remove fuzz.

I double checked that the change doesn't break memcached documented protocol:

https://github.com/memcached/memcached/blob/master/doc/protocol.txt

and it seems to be okay, but I'm not an expert.

I tested it on a live trusty system by:

% telnet localhost 11211
Trying 127.0.0.1...
Connected to localhost.
Escape character is '^]'.
version
VERSION 1.4.14 (Ubuntu)
quit
Connection closed by foreign host.

So it looks good to me and I'll sponsor the package with these changes. Thanks!

review: Approve

« Back to merge proposal