Skip to content
This repository has been archived by the owner on Jan 21, 2020. It is now read-only.

Feature/enable zf development mode just like in ZendSkeletonApp #105

Conversation

gabbydgab
Copy link

Based on my suggestion in #104

Following changes are:

  • updated composer.json with zfcampus/zf-development-mode and scripts
  • added development.config.php.dist file (including .gitignore)
  • updated the merged configuration if development-mode is enabled.

@gabbydgab
Copy link
Author

Should we include the error_reporting?

// when all else fails, kindly show me the bugs :)
if ($config['debug']) {
    error_reporting(E_ALL);
    ini_set("display_errors", 1);
}

Purpose: Making sure that what you commit as production code is clean and Uncle Bob will be happy 👍 😂

@geerteltink
Copy link
Member

Should we include the error_reporting?

You've got whoops to catch errors.

Things that needs to be addressed:

  • The config cache is not removed when enabling / disabling debug mode.
  • Why is development.config.php saved in ./config? I'd rather see it saved in ./config/autoload/development.local.php.
  • If an user chooses not to install whoops, it shouldn't be forced and left out of development.config.php.dist and composer.json.
  • Same goes for minimal setup. Whoops and zf-development-mode should not be installed.

Maybe we should rename the whoops option to development tools. Or make whoops an optional package and let the installer handle it (together with the ConfigProvider and component installer).

// Development mode enabled
if (file_exists(__DIR__ . '/../config/development.config.php')) {
$config = ArrayUtils::merge($config, require __DIR__ . '/../config/development.config.php');
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would place this after the block that caches configuration; we likely should never cache development configuration.

@weierophinney
Copy link
Member

I agree with all points that @xtreamwayz notes. If we get those resolved, I'd be happy to incorporate this, however!

@geerteltink
Copy link
Member

@gabbydgab Thanx for this. This feature is included in #54.

@weierophinney weierophinney modified the milestones: 1.1.0, 2.0.0 Jan 26, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants