[Config] Allow using defaultNull() on BooleanNodeDefinition
#58490
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
69acbf9 to
71bace0
Compare
stof
reviewed
Oct 8, 2024
9f8f8f1 to
2948789
Compare
|
Do we need some changes in config builders, like done in #58428? |
2948789 to
06d0e19
Compare
06d0e19 to
6aad690
Compare
|
I don't think so, |
nicolas-grekas
approved these changes
Oct 9, 2024
|
Thank you @alexandre-daubois. |
javiereguiluz
added a commit
to symfony/symfony-docs
that referenced
this pull request
Oct 16, 2024
This PR was merged into the 5.4 branch. Discussion ---------- Add BC promise rules for constructors As spotted in symfony/symfony#58490, following symfony/symfony#58490 (comment) Commits ------- 72d416c Add BC promise rules for constructors
Merged
|
This change apparently broke MonologBundle's test suite, see https://github.com/symfony/monolog-bundle/actions/runs/11651735613/job/32442060641#step:8:25 for instance. Can you please have a look, @alexandre-daubois? |
|
Yes 👍 |
derrabus
added a commit
to symfony/monolog-bundle
that referenced
this pull request
Nov 6, 2024
This PR was merged into the 3.x-dev branch. Discussion ---------- Fix test with `symfony/config:^7.2` Following symfony/symfony#58490 (comment). Until the Symfony PR, null wasn't supported in boolean nodes and defaulted to true. Commits ------- 3fd3da1 Fix test with `symfony/config:^7.2`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
After this comment. Currently,
defaultNull()on BooleanNode doesn't work well: the value always casts totrue. This PR allows to make the node nullable by callingdefaultNull()ordefaultValue(null).Given the following configuration on a brand new project (with
composer require symfony/form symfony/csrf-protection):Dumping
$config['form']['csrf_protection']['enabled']reveals theenabledoption is casted totruewhen set tonullin the config. The following condition is never true:symfony/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
Line 755 in b61db2f
With this PR, the
enabledoption keeps thenullvalue when defining the node as it already is in the code:symfony/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php
Line 214 in b61db2f