8000 [FrameworkBundle] Fix framework bundle lock configuration not working as expected by HypeMC · Pull Request #31198 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 26, 2019
Merged

[FrameworkBundle] Fix framework bundle lock configuration not working as expected #31198

merged 1 commit into from
Sep 26, 2019

Conversation

HypeMC
Copy link
Member
@HypeMC HypeMC commented Apr 22, 2019
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#11466

This fixes #31197 and makes the lock configuration work with installations that are not full stack ones and configurations that use xml files.

@nicolas-grekas
Copy link
Member

ping @jderusse

Copy link
Member
@jderusse jderusse left a 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-implemented fixXmlConfig + 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.

@HypeMC
Copy link
Member Author
HypeMC commented Apr 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
Copy link
Member
stof commented Jul 18, 2019

I would rather add a check for the singular resource key in the existing normalization condition rather than reimplementing a 8000 ll other features.

@HypeMC
Copy link
Member Author
HypeMC commented Jul 25, 2019

@stof Tried your approach & you were right, the end result is now a bit cleaner HypeMC@662d394 . Not sure whether to update this PR since it's already approved or to open a new one.

@nicolas-grekas
Copy link
Member

Please update!

@HypeMC
Copy link
Member Author
HypeMC commented Jul 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']); })
Copy link
Member

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?

Copy link
Member

Choose a reason for hiding this comment

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

oh no, extra s...

@nicolas-grekas
Copy link
Member

Thank you @HypeMC.

nicolas-grekas added a commit that referenced this pull request Sep 26, 2019
…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#11466

This fixes #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
@nicolas-grekas nicolas-grekas merged commit c7af2df into symfony:3.4 Sep 26, 2019
This was referenced Oct 7, 2019
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.

7 participants
0