Re: New pecl project

Hi Johannes On 09/04/15 20:30, Johannes Schlüter wrote: > Hi, > > On Wed, 2015-04-08 at 04:08 -0400, Cesar Rodas wrote: >> I'm working on a project to embed duktape (Javascript) engine inside >> PHP. What are the requirements to host the project on pecl? > The requirement is to write a mail as you did and hope somebody reviews > it ;-) > > A few review comments: > > 1. Please add a license header, especially as the repo mixes code > from different origin (duktape and your code) > 2. Please add /* {{{ proto */ comments to exported functions, this > helps documentation tools > 3. Please don't use zend_error, see > > https://wiki.php.net/internals/review_comments#don_t_use_zend_error > 4. E_ERROR typically is a bad choice, should only be used when you > really can't recover, maybe E_RECOVERABLE or an exception are > better > 5. Provide a php_phpjs.h header (this is needed for static builds, > which probably nobody will do, but is common style, see > > https://wiki.php.net/internals/review_comments#php_extnameh_should_be_minimal) > 6. We prefer a coding style where even single line if's etc. have > {} (helps to prevent bugs like Apple's goto fail;) While we're > not strict about this I was lazy, and php2c.php generated that, I will surely fix and add {} everywhere! > > All minor things, but I also didn't do an in-depth review ;-) > If those are fixed seems ok to me. Thanks for the feedback. When the project is cleaner and with more tests I will send another request for feedback (and approval!) Cheers, > > johannes -- César D. Rodas Open Source developer +595-983-161124 PGP: F9ED A265 A3AB C8A1 D145 7368 158A 0336 C707 0AA6

Attachment: [application/pgp-signature] OpenPGP digital signature signature.asc