Re: Request to submit Stackdriver Trace PHP extension to PECL

Thanks! I'll make those code changes and submit the account request. - Jeff Ching On Thu, Jul 20, 2017 at 8:19 AM, Johannes Schlüter <johannes@php.net> wrote: > 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 >