Re: Request to submit Stackdriver Trace PHP extension to PECL
| From: | Johannes Schlüter | Date: | Thu, 20 Jul 2017 15:19:42 +0000 |
| Subject: | Re: Request to submit Stackdriver Trace PHP extension to PECL | ||
| References: | 1 | Groups: | php.pecl.dev |
| Request: | Send a blank email to pecl-dev+get-14737@lists.php.net to get a copy of this message | ||
Hi,
On Mo, 2017-07-17 at 14:18 -0700, Jeff Ching wrote:
>
> I previously sent a request for this, but now the code is available
> at
> https://github.com/GoogleCloudPlatform/stackdriver-trace-php-
> extension. It is currently tested against the latest versions of 7.0
> and 7.1, as well as against 7.2.0alpha3.
I had a quick look and see no major issue. Small comments:
- We prefer BSD/MIT license, but Apache License 2 should be acceptable.
- The .c files have C++/C99 comments (// instead of /* */) this can
lead to issues in stricter compilers (at least some versions of visual
stdio)
- Our default coding style marks PHP_FUNCTIONS with "proto" comments as
there are/were tools by the doc team to extract those (while I believe
modern docgen scripts use reflection)
In PHP_METHOD(StackdriverTraceSpan, __construct) I see a RETURN_FALSE
on argument parsing failure, coding style is a plain "return;" which
leads to a NULL return value ... but in a constructor this doesn't
really matter as the value is ignored :-)
I haven't tested or tried to find logic errors or such, but think you
can go ahead and register an account on https://pecl.php.net/account-re
quest.php
johannes