Thanks for the feedback Gustavo. New patch is here: https://gist.github.com/1582997 Test case: https://gist.github.com/1583022
Comments are inline. On Mon, Jan 9, 2012 at 8:17 AM, Gustavo Lopes <glo...@nebm.ist.utl.pt> wrote: > On Mon, 09 Jan 2012 04:59:09 +0100, Paul Dragoonis <dragoo...@gmail.com> > wrote: > >> Hey Internals, >> >> I've finished the patch, and with approval i'd like to push to trunk, >> even though i'm aware we have a 5_4 branch code freeze. >> >> Can someone review my work and provide feedback/approval? >> >> [1] https://gist.github.com/1580974 >> > > Just a few comments: > > * There's zend_parse_parameters_none, which basically expands to what you've > done. I was aware of this, the entire class is using the old method not _none() so i guess as a separate commit I could update this class to use that. I have now added _none() to my patch. > * We use tabs for indentation not space (tabstop 4) Resolved indentation. > * Leave a space between "while" and the parenthesis. > * Your declarations should be on the top of a block, otherwise compilers > that don't implement that C99 feature (like Microsoft's Visual C++) will > reject them. I've moved my declarations to the top. > * In C (though not in C++), you don't have to add casts on conversions from > void*. This is more of an opinion, but I also think you shouldn't because > otherwise if you forget an include, you won't get a warning on the > conversion of the implicit int return of undeclared functions to your target > pointer type. > > -- > Using Opera's revolutionary email client: http://www.opera.com/mail/ > > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php > -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
