[PEPr] Comment on Configuration::Config_Lite
| From: | Christian Weiske | 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