IniParser refactoring, filename prefixes, test fixes #104
Conversation
There was a problem hiding this comment.
Thanks for your PR! Sorry for the late answer. Can you please address the requested changes? Maybe @hassankhan could add some tips for this PR.
| * | ||
| * @throws EmptyDirectoryException If `$path` is an empty directory | ||
| */ | ||
| public function __construct($path) | ||
| public function __construct($path, $prefix = false) |
There was a problem hiding this comment.
Can you please use an associative array with the configuration instead of having $prefix directly as second parameter?
There was a problem hiding this comment.
Just to clarify, the constructor method should look like __construct($path, $options = []) where prefix would be a key under $options.
| * | ||
| * @throws EmptyDirectoryException If `$path` is an empty directory | ||
| */ | ||
| public function __construct($path) | ||
| public function __construct($path, $prefix = false) |
There was a problem hiding this comment.
Just to clarify, the constructor method should look like __construct($path, $options = []) where prefix would be a key under $options.
|
Hi @kekenec, thanks for the PR! Just curious as to whether there was a specific reason for changing the QCNs in the test files? |
|
This PR has the conflicted files and it should be fixed. |
| */ | ||
| protected $data = null; | ||
| protected $data = array(); |
There was a problem hiding this comment.
How about setting the short array syntax []?
| * @covers \Noodlehaus\Config::__construct() | ||
| * @covers \Noodlehaus\Config::get() | ||
| */ | ||
| public function testConstructWithPrefix() { |
There was a problem hiding this comment.
This method code block should be:
public function testConstructWithPrefix()
{
......
}
$dataequals tonullandparse_ini_filedidn't throw any error/warning. So when we callerror_get_lastwe assume that there wasn't errors in the past that are suppressed via@/custom error handler. But even PHPUnit uses@operator, e.g. for creating log directories and files specified in configuration file, and it may cause warning/error while testing.$prefixforConfig::__constructmethod. If$prefixis true, then every configuration file provided in the first argument will be parsed as an array containing the config array itself by the filename as a key.I'm sorry for this huge pull request. If needed, i can break it up to several small requests.