laminas-api-tools / api-tools

Laminas API Tools module for Laminas
https://api-tools.getlaminas.org/documentation
BSD 3-Clause "New" or "Revised" License
37 stars 19 forks source link

Respect existing config.php and local.php #25

Open weierophinney opened 4 years ago

weierophinney commented 4 years ago

Apigility should write to its own config files rather than rewriting existing files. We've had to move the application's config out of global.php and into app.global.php because Apigility kept messing it up each time a change was made in the admin ui. Eg. closures would break, and constants were being replaced with their values.


Originally posted by @dkmuir at https://github.com/zfcampus/zf-apigility/issues/166

weierophinney commented 4 years ago

This is just my opinion, not that of Apigility.

You should never use closures in your config files and you should never use constants outside very specific-use classes.

The problems you listed are not because Apigility uses the expected global.php file and it doesn't modify the local.php file. These problems are because you're using config files wrong.

For anything which requires a fixture put it in the service manager config of the module. For anything that requires a constant drag the developer out by his hair.


Originally posted by @TomHAnderson at https://github.com/zfcampus/zf-apigility/issues/166#issuecomment-224456558

weierophinney commented 4 years ago

local.php was modified to include zf-mvc-auth. I agree that closures are not the best in config files (eg, breaks caching the merged file), so we're converting those into factories. I completely disagree with your opinion of constants. Which would you rather see?

'error_reporting' => 32767,
...
'driver_options' => array(
    1002 => 'SET NAMES \'UTF8\''
),
...
'curloptions' => array(
    64 => true,
),

or:

'error_reporting' => E_ALL,
...
'driver_options' => array(
    PDO::MYSQL_ATTR_INIT_COMMAND => 'SET NAMES \'UTF8\''
),
...
'curloptions' => array(
    CURLOPT_SSL_VERIFYPEER => true,
),

Originally posted by @dkmuir at https://github.com/zfcampus/zf-apigility/issues/166#issuecomment-224465050

weierophinney commented 4 years ago

$serviceManager->get('doctrine.entitymanager.orm_default');


Originally posted by @TomHAnderson at https://github.com/zfcampus/zf-apigility/issues/166#issuecomment-224466611

weierophinney commented 4 years ago

For error reporting that should be a function in local.php set_error_reporting and it should use the PHP E_ALL constant. My previous comment I hope shows my ignorance of PDO. I think you have a point about Apigility changing local.php.


Originally posted by @TomHAnderson at https://github.com/zfcampus/zf-apigility/issues/166#issuecomment-224467246