Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added Mezzio development config discovery and injection #68

Merged
merged 1 commit into from
Jul 17, 2023

Conversation

kynx
Copy link

@kynx kynx commented Mar 10, 2023

Q A
Documentation no
Bugfix no
BC Break no
New Feature yes
RFC no
QA no

Description

This PR adds support for injecting config providers into Mezzio's config/development.config.php.dist, following the recommendations from @boesing on mezzio/mezzio-tooling#25. This is useful when adding --dev tools that shouldn't be shipped in production.

If merged a separate PR will be needed against the Mezzio skeleton so the development config looks something like:

declare(strict_types=1);

use Laminas\ConfigAggregator\ConfigAggregator;

$aggregator = new ConfigAggregator([
    function() {
        return [
            'debug'                        => true,
            ConfigAggregator::ENABLE_CACHE => false,
        ];
    }
]);

return $aggregator->getMergedConfig();

Existing applications that do not have this development config will continue to inject only to the main config/config.php.

Signed-off-by: matt <matt@claritum.com>
@@ -18,16 +18,14 @@ final class ConfigAggregatorInjector extends AbstractInjector
{
use ConditionalDiscoveryTrait;

/** @var non-empty-string */
Copy link
Member

Choose a reason for hiding this comment

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

Constants do not need types as they're constant and thus inferred anyway.

Copy link
Member

@boesing boesing left a comment

Choose a reason for hiding this comment

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

Works for me. Sorry for the late feedback, had a bunch of other stuff todo...

@boesing
Copy link
Member

boesing commented Jul 7, 2023

@kynx did you used this PR in a project to actually verify integrative functionality? I do not have time for it, but I am not 100% sure from reviewing the PR if the installer is actually asking for adding it either to the development or production configuration.

@kynx
Copy link
Author

kynx commented Jul 14, 2023

@boesing Yes I've verified it works. If you've got time, give this a try:

git clone git@github.com:kynx/component-installer-demo.git
cd component-installer-demo
composer install

That's a minimal version of mezzio skeleton with mezzio/mezzio-tooling removed. Then:

composer require --dev mezzio/mezzio-tooling
<snip>

  Please select which config file you wish to inject 'Mezzio\Tooling\ConfigProvider' into:
  [0] Do not inject
  [1] config/development.config.php.dist
  [2] config/config.php
  Make your selection (default is 1):1

  Remember this option for other packages of the same type? (Y/n)
    Installing Mezzio\Tooling\ConfigProvider from package mezzio/mezzio-tooling

Selecting 1 has added the ConfigProvider to development.config.php.dist:

git --no-pager diff config
diff --git a/config/development.config.php.dist b/config/development.config.php.dist
index e6a4249..9917504 100644
--- a/config/development.config.php.dist
+++ b/config/development.config.php.dist
@@ -30,6 +30,7 @@ use Laminas\ConfigAggregator\ConfigAggregator;
 use Laminas\ConfigAggregator\PhpFileProvider;
 
 $aggregator = new ConfigAggregator([
+    \Mezzio\Tooling\ConfigProvider::class,
     new PhpFileProvider(realpath(__DIR__) . '/autoload/{,*.}{global,local}-development.php'),
     new ArrayProvider([
         'debug'                        => true,

Note that this does require there to be a ConfigAggregator in that file - as per mezzio/mezzio-skeleton#122

@boesing boesing merged commit e1ceb9a into laminas:3.3.x Jul 17, 2023
@boesing
Copy link
Member

boesing commented Jul 17, 2023

LGTM, lets ship this.
Thx, @kynx

@boesing boesing added this to the 3.3.0 milestone Jul 17, 2023
@kynx kynx deleted the mezzio-development-config branch July 17, 2023 09:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants