Re: Proposal Auth_WIKID Extra Comment (QA)

done! On 4/6/07, David Coallier <davidc@php.net> wrote:

Do you think you could upload your new version to http://dev.wikidsystems.com/docs/php/pear/WiKID.phps ? Thanks :) On 4/6/07, Greg Haygood <ghaygood@gmail.com> wrote: Thanks!. I've updated my code with all your suggestions but the last: I've set the dependency on the openssl extension in package{,2}.xml and on the project page - do I also need to specify it in the source or somewhere else? On 4/5/07, David Coallier <davidc@php.net> wrote:

Hello all, this is an email to do extra QA on http://pear.php.net/pepr/pepr-proposal-show.php?id=321 First of all Greg, good work on the package, I really like the overall structure, the code is very clean, and I like the comments themselves and it looks good. However, I must add a couple points: The package is noted as PHP5, so the variable properties should be respected. I see in the documentor's code that all the variables access is noted as @access private, therefore, you should usually make the variables private $_isConnected instead of var $_isConnected; Because by default var is now E_STRICT compliant, however it will make the variables public. Also, making the variables private is not recommended as it makes them less flexible, therefore it is recommended to use * protected for pear packages. Next point: In the constructor of your class (which is not defined using __construct() { }) I see that you are doing :

       $min_version = "5.0.0";
        if (version_compare($min_version, phpversion(), ">=")) {

            echo "Incompatible version - Auth_WiKID requires PHP >=

$min_version\n";

            return null;
        }

This is overhead because the pear installer is handling all this using the package.xml file (dependencies). Next point: In php5, you can use __destruct() in order to execute the destroy and all your close() function. Next point: As stated you have to make a dependency for openssl Next point: Nothing else to say else than good work for this quality package :) Thanks, P.S. Any other input would be appreciated guys.. (wink)(wink) -- David Coallier, Founder & Software Architect, Agora Production (http://agoraproduction.com) 51.42.06.70.18

-- Greg Haygood ghaygood@gmail.com -- David Coallier, Founder & Software Architect, Agora Production (http://agoraproduction.com) 51.42.06.70.18

-- Greg Haygood ghaygood@gmail.com