Re: Adding libmaxminddb extension to PECL

Thanks for the feedback! > > * Your license seems to be LGPL2.1. This is fine but we prefer PHP > License 3.01 or BSD for extensions. > I think we will be able to accommodate this. > * We try to be C89 compatible which requires declaring variables > first on top of a block see > > > https://wiki.php.net/internals/review_comments#don_t_use_c99_for_portability_reasons > * Your manipulation of CFLAGS in config.m4 prevents from > compilation with other compilers, either detect whether the > compiler supports these or let the user set the while running > configure. > * The config.m4 links against an external library, as a guideline > in such a case we use "--with-foo" not "--enable-foo" as option. > By using "--with-foo" the user could pass the location of the > library in case it was installed in a non-standard path > * the config.w32 file doesn't link that library at all, either it > should be made working (Pierre or Anatol probably might help?) > or removed * Your php-maxminddb header includes quite a few other headers, > this is discouraged, see > I'll fix the above. > > > https://wiki.php.net/internals/review_comments#php_extnameh_should_be_minimal > * maxminddb_ce->ce_flags |= ZEND_ACC_FINAL | ZEND_ACC_ABSTRACT; > this seems weird, final and abstract!? > Yeah, that does seem wrong. :-) I'll fix it. Greg