Re: Become a mantainer of wxphp

On Mon, 2012-04-30 at 13:46 -0400, JGM wrote: > Hi, > > I'm jefferson known as jgmdev when I connect to the #php.pecl to ask > questions :) > > I'm writing because I would like to maintain the code of wxphp extension, > I'm already maintaining it but on the following sourceforge project page: Did you talk to Mário when taking it over? Is he ok with that? Is there a "migration path" for people coming from the old version to the new one? Will everything still work? (not needed, but useful ...) > http://sourceforge.net/projects/wxphp/ > > *svn co http://wxphp.svn.sourceforge.net/svnroot/wxphp/trunk > wxphp* I had a *quick* look at some code files. Some small comments: If PHP_METHOD(php_wxApp, Yield) in app.cpp is a hand-written it is strange (for instance it sets "valid=1" and then does "if (valid)"it also does an "if (ZEND_NUM_ARGS()==0)" followed by a switch based on the same value. Functions not expecting parameters should use zend_parse_parameters_none() to produce a nicer error. zend_parse_parameters is always set to quiet mode, therefore the user won't get a proper error when passing wrong arguments. The PHP_METHODs should have some "proto" comment. This helps with some documentation tools and while reading the code. You shouldn't use zend_error outside the engine, but use php_error_docref which gives abit nicer error messages. Preprocessor macros should always start in the first column. You're using call_user_function_ex() the zend_call_method[_with_[1-3]_param[s] family of functions from zend_interfaces.h can be nicer. You might mark your strings (zend_parse_parameters flags or stuff like char _wxResource[] = "wxResource";) as const sothe compiler can create better code and catch errors. (maybe I'm wrong and PHP/Zend doesn't define the function params const .. then that should be fixed ;-) ) I think bool wxLogWindow_php::OnFrameClose(wxFrame* frame) won't free arguments[0] and [1] properly. Did you check for leaks? Why are you using objects and resources? The CREDITS file should list all names in one line (used afaik by win build utils and for bundled extensions to create PHP credits) Generated code is hard to read ;-) johannes