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

Reply via email to