Hello Ronen,
Just a few comments:
1) it's A LOT of code
2) most of it doesn't seem to conform to the coding standard we're used to:
goto's everywhere, strange things like
if (PHP_TYPE_ISNOTARR(key_record_p) ||
(!bin_name_p) || (!prepend_str_p) ||
((options_p) && (PHP_TYPE_ISNOTARR(options_p)))) {
That's quite a lot of unnecessary brackets.
Also the check itself doesn't make a lot of sense to me: these variables aren't
optional, so they cannot be NULL anyway.
Similarly, in many places you request arrays using zend_parse_parameters(ZEND_NUM_ARGS()
TSRMLS_CC, "a") and then check if they are really arrays. There's no point in
that.
3) Returning long on error (especially on parameters parsing error) I'd call a
bad idea - there's no way to distinguish between the 'real' errors and
programmer's errors this way.
4) Do you really need to duplicate the code this way?
PHP_EXT_SET_AS_ERR(&error, AEROSPIKE_ERR_PARAM, "Unable to parse php
parameters for append function");
DEBUG_PHP_EXT_ERROR("Unable to parse key parameters for append
function");
DEBUG_PHP_EXT_ERROR() can be added to PHP_EXT_SET_AS_ERR() instead.
Btw, these _ERROR() and _ERR() suffixes should probably be consistent if you
insist on using these macros.
5) The extension doesn't compile:
In file included from /local/git/php-src/ext/aerospike/aerospike.c:35:0:
/local/git/php-src/ext/aerospike/php_aerospike.h:35:32: fatal error:
aerospike/as_error.h: No such file or directory
#include "aerospike/as_error.h"
I guess it requires some library, but it doesn't look or check for it during
configure because it uses --enable-aerospike instead of
--with-aerospike=[INSTALL PREFIX of the lib].
On 10/17/2014 08:13 AM, Ronen Botzer wrote:
Hello,
It's been over a week since I introduced the PHP extension for Aerospike (
http://news.php.net/php.pecl.dev/12321). So far there have been no
comments for or against. I'd appreciate if someone took a look at it.
Subsequently, I would need an account as the maintainer.
Aerospike is an open-source NoSQL database, not unlike Redis or Couchbase
or Mongo, all of which are already in PECL. The code is currently on
GitHub (https://github.com/aerospike/aerospike-client-php) and available
through Composer (
https://packagist.org/packages/aerospike/aerospike-client-php).
Thanks,
Ronen
--
Wbr,
Antony Dovgal
---
http://pinba.org - realtime profiling for PHP
--
PECL development discussion Mailing List (http://pecl.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php
