Skip to content

[Config] Allow to always use config classes #51544

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

Draft
wants to merge 2 commits into
base: 7.3
Choose a base branch
from

Conversation

alexndlm
Copy link
Contributor

@alexndlm alexndlm commented Sep 4, 2023

Q A
Branch? 6.3
Bug fix? yes
New feature? no
Deprecations? no
License MIT

When the config node doesn't have normalization, using the config object is impossible.

When the config node has some array normalization (like http_codes in \Symfony\Bundle\FrameworkBundle\DependencyInjection\Configuration::addHttpClientRetrySection), it is not possible to use the config object.

This PR relates to changes introduced in:

Снимок экрана 2023-09-01 в 22 22 28

@nicolas-grekas
Copy link
Member

Thanks for working on this, it's related to the limitation I found in #51273
Can you please rebase+target 6.4? That's an improvement, not a bugfix to me.

Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

The changes on the Config components are BC breaks. Can we find a way that doesn't break BC?

@@ -374,7 +374,7 @@ protected function createNode(): NodeInterface

if ($this->default) {
if (!\is_array($this->defaultValue)) {
throw new \InvalidArgumentException(sprintf('%s: the default value of an array node has to be an array.', $node->getPath()));
throw new \InvalidArgumentException(sprintf('"%s": the default value of an array node has to be an array.', $node->getPath()));
Copy link
Member

Choose a reason for hiding this comment

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

should be reverted, that's a false-positive from fabbot I suppose

protected $node;

public $allowedTypes;
Copy link
Member

Choose a reason for hiding this comment

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

removing consts and public properties is a BC break, we need to figure out a way that doesn't break BC

@alexndlm alexndlm force-pushed the feature/config-allow-to-always-use-config-classes branch from f4bb242 to 72e1fd0 Compare November 11, 2023 14:08
@alexndlm alexndlm marked this pull request as draft November 11, 2023 14:13
@alexndlm alexndlm changed the base branch from 6.3 to 6.4 November 11, 2023 14:13
@nicolas-grekas nicolas-grekas modified the milestones: 6.3, 6.4 Feb 1, 2024
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.

4 participants