[PEPr] Comment on Configuration::Config_Lite

From: Date: Wed, 05 Jan 2011 08:28:06 +0000
Subject: [PEPr] Comment on Configuration::Config_Lite
References: 1  Groups: php.pear.dev 
Request: Send a blank email to pear-dev+get-53938@lists.php.net to get a copy of this message
- run phpcs on the code - rename AllTests.php -> Config_ListeTest.php. AllTests in PEAR is to include these specific files only, not to contain any tests itself - Exceptions in Exception subfolder: Config_Lite_Exception_Runtime - docblocks should be more verbose, i.e. get(): "get" as description is a bit short. "Section" param doc should contain a hint about null values. $default param doc should state that it's used when the config key does not exist - I'd get rid of hasSection() and make the $key in has() optional, same for remove/removeSection - cweiske:~/Dev/pear/sandbox/config_lite/Config> php ../tests/AllTests.php PHP Notice: Please no longer include "PHPUnit/Framework.php". in /usr/share/php/PHPUnit/Framework.php on line 50 - cweiske:~/Dev/pear/sandbox/config_lite/Config> phpunit ../tests/AllTests.php EEEEEEEEEEEEEEEEEEEEEE There were 22 errors: Config_Lite_RuntimeException: file not found: test.cfg -> you need to use dirname(__FILE__) to get the correct test.cfg path when running unit tests - tests/test.cfg is changed during the unit tests, but is also used as test fixture. This is not a good idea. Temporary config files should be stored in sys_get_temp_dir() - "no filename given" is a bit incorrect in sync(), since there is no filename parameter. - you may not register your own autoloader. either propse the package in pear2 and expect the class to exist, or require_once the files you need as all pear packages do. throwing an exception when a file is included is also not a good idea. -- http://pear.php.net/pepr/pepr-proposal-show.php?id=645

« previous php.pear.dev (#53938) next »