Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[FrameworkBundle] Fix framework bundle lock configuration not working as expected#31198
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
[FrameworkBundle] Fix framework bundle lock configuration not working as expected#31198
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Uh oh!
There was an error while loading.Please reload this page.
nicolas-grekas commentedApr 23, 2019
ping@jderusse |
jderusse left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I'm not an expert on symfony configuration, but it's look like we've re-implementedfixXmlConfig +useAttributeAsKey('name') and got the feeling that business logic is dupliacted in "resource" and "resources".
Side appart that comment, I'm 👍 with the test suite: which is the expected behavior.
src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
HypeMC commentedApr 23, 2019
@jderusse Yep, I agree with you, this is pretty much a re-implementation of fixXmlConfig + useAttributeAsKey('name'), however because of the beforeNormalization rules that were here before, I haven't been able to find a better solution. If anyone has any ideas please, let me know. Regardless, the fact remains that currently xml configurations don't work at all. |
stof commentedJul 18, 2019
I would rather add a check for the singular |
HypeMC commentedJul 25, 2019
@stof Tried your approach & you were right, the end result is now a bit cleanerHypeMC@662d394 . Not sure whether to update this PR since it's already approved or to open a new one. |
nicolas-grekas commentedJul 25, 2019
Please update! |
HypeMC commentedJul 25, 2019
@nicolas-grekas Done. |
| ->then(function ($v) {return$v + ['enabled' =>true]; }) | ||
| ->end() | ||
| ->beforeNormalization() | ||
| ->ifTrue(function ($v) {return\is_array($v) && !isset($v['resources']) && !isset($v['resource']); }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
&& !isset($v['resources']) is duplicated right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
oh no, extras...
nicolas-grekas commentedSep 26, 2019
Thank you@HypeMC. |
…not working as expected (HypeMC)This PR was squashed before being merged into the 3.4 branch (closes#31198).Discussion----------[FrameworkBundle] Fix framework bundle lock configuration not working as expected| Q | A| ------------- | ---| Branch? | 3.4| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#31197| License | MIT| Doc PR |symfony/symfony-docs#11465 &symfony/symfony-docs#11466Thisfixes#31197 and makes the lock configuration work with installations that are not full stack ones and configurations that use xml files.Commits-------c7af2df [FrameworkBundle] Fix framework bundle lock configuration not working as expected
Uh oh!
There was an error while loading.Please reload this page.
Thisfixes#31197 and makes the lock configuration work with installations that are not full stack ones and configurations that use xml files.