Skip to content
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

[DependencyInjection] Fixed the getServiceIds implementation to always return aliases #33335

Merged
merged 1 commit into from Aug 26, 2019

Conversation

pvandommelen
Copy link
Contributor

Q A
Branch? 3.4
Bug fix? yes
New feature? no
BC breaks? maybe
Deprecations? no
Tests pass? yes
Fixed tickets #33307
License MIT
Doc PR -

Changed the getServiceIds implementation in the Container base class to include aliases. Modified existing tests. Added test which uses the PhpDumper.
Fixes #33307

Without this patch the implementations of the container are inconsistent in whether or not they return aliases (see issue). Fixing this could be considered a BC break for the affected Container class.

As an alternative to keep the behaviour in Container unchanged, the dumped container could be patched instead. And then only apply this version of the patch to master. This however keeps the inconsistency between Container and ContainerBuilder.

@stof
Copy link
Member

stof commented Aug 26, 2019

why is it a draft PR ? What is left to do with it ?

@pvandommelen
Copy link
Contributor Author

Nothing, wanted to check it over myself briefly.

@pvandommelen pvandommelen marked this pull request as ready for review August 26, 2019 09:19
@nicolas-grekas nicolas-grekas added this to the 3.4 milestone Aug 26, 2019
@fabpot
Copy link
Member

fabpot commented Aug 26, 2019

Thank you @pvandommelen.

fabpot added a commit that referenced this pull request Aug 26, 2019
…tion to always return aliases (pdommelen)

This PR was squashed before being merged into the 3.4 branch (closes #33335).

Discussion
----------

[DependencyInjection] Fixed the `getServiceIds` implementation to always return aliases

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | maybe
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #33307
| License       | MIT
| Doc PR        | -

Changed the getServiceIds implementation in the Container base class to include aliases. Modified existing tests. Added test which uses the PhpDumper.
Fixes #33307

Without this patch the implementations of the container are inconsistent in whether or not they return aliases (see issue). Fixing this could be considered a BC break for the affected Container class.

As an alternative to keep the behaviour in Container unchanged, the dumped container could be patched instead. And then only apply this version of the patch to master. This however keeps the inconsistency between Container and ContainerBuilder.

Commits
-------

834d5cb [DependencyInjection] Fixed the `getServiceIds` implementation to always return aliases
@fabpot fabpot merged commit 834d5cb into symfony:3.4 Aug 26, 2019
This was referenced Aug 26, 2019
@pvandommelen pvandommelen deleted the fix-getserviceids branch August 27, 2019 12:30
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.

None yet

6 participants