Skip to content

Conversation

mmarchois
Copy link

The load method definition seems to be wrong. In the Symfony\Component\DependencyInjection\ContainerBuilder\ExtensionInterface, the method definition is public function load(array $config, ContainerBuilder $container);

The `load` method definition seems to be wrong. In the `Symfony\Component\DependencyInjection\ContainerBuilder\ExtensionInterface`, the method definition is `public function load(array $config, ContainerBuilder $container);`
@mmarchois mmarchois changed the title Load method definition from ExtensionInterface load() method definition from ExtensionInterface Dec 8, 2015
@ogizanagi
Copy link
Contributor

I wonder if we shouldn't fix the ExtensionInterface::load signature instead.

Every extensions defined in Symfony's bundles for instance (but you can check other ones, as well as third party bundles) are using $configs. And actually that makes a lot of sense, as $configs is an array of extension configs, not already merged, on contrary of the processed configuration you usually get by:

$config = $this->processConfiguration(new Configuration(), $configs);

In other words, for the framework extension, you can actually get something like this value for $configs:

array:2 [▼
  0 => array:13 [▼
    "translator" => array:1 [▶]
    "secret" => "ThisTokenIsNotSoSecretChangeIt"
    "router" => array:2 [▼
      "resource" => "[..]/app/config/routing.yml"
      "strict_requirements" => null
    ]
    "form" => null
    "csrf_protection" => null
    "validation" => array:1 [▶]
    // [...]
  ]
  1 => array:2 [▼
    "router" => array:2 [▼
      "resource" => "[..]/app/config/routing_dev.yml"
      "strict_requirements" => true
    ]
    "profiler" => array:1 [▼
      "only_exceptions" => false
    ]
  ]
]

because the framework extension key is used many (2) times: in app/config/config.yml and app/config/config_dev.yml. The values are not merged yet. This is the task of the symfony/config component.

@wouterj
Copy link
Member

wouterj commented Dec 17, 2015

I think we explicitely renamed it to $configs to better cover the value of this parameter: A list of the configuration (which has yet to be merged/processed into a single config).

So I'm going to close this one and I recommend you open a PR/issue on the code repository to rename the parameter instead. If you don't agree, feel free to leave a comment!

@wouterj wouterj closed this Dec 17, 2015
fabpot added a commit to symfony/symfony that referenced this pull request Dec 26, 2015
… method definition (mmarchois)

This PR was submitted for the 3.0 branch but it was merged into the 2.3 branch instead (closes #17131).

Discussion
----------

[DependencyInjection] Change the ExtensionInterface load method definition

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | ~
| License       | MIT
| Doc PR        | symfony/symfony-docs#5988

This PR change the `Symfony\Component\DependencyInjection\Extension\ExtensionInterface` load method definition to be identical to the documentation :
`public function load(array $configs, ContainerBuilder $container);`

Commits
-------

85c271b Change the ExtensionInterface load method definition to bo identical to the documentation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants