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

[Di] Remove closure-proxy arguments #23022

Merged
merged 1 commit into from Jun 2, 2017

Conversation

nicolas-grekas
Copy link
Member

@nicolas-grekas nicolas-grekas commented Jun 1, 2017

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

With #23008, we don't need this in core anymore.
Let's drop it now.
Technically that's a BC break, but for a feature that is very new, and still quite hidden.
Doing this now would save us from maintaining this code, and help reduce the overall complexity.

Basically reverts #20953

@nicolas-grekas
Copy link
Member Author

and green :)

@lyrixx
Copy link
Member

lyrixx commented Jun 2, 2017

👍

@nicolas-grekas nicolas-grekas merged commit 57daadb into symfony:3.3 Jun 2, 2017
nicolas-grekas added a commit that referenced this pull request Jun 2, 2017
This PR was merged into the 3.3 branch.

Discussion
----------

[Di] Remove closure-proxy arguments

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

With #23008, we don't need this in core anymore.
Let's drop it now.
Technically that's a BC break, but for a feature that is very new, and still quite hidden.
Doing this now would save us from maintaining this code, and help reduce the overall complexity.

Basically reverts #20953

Commits
-------

57daadb [Di] Remove closure-proxy arguments
@nicolas-grekas nicolas-grekas deleted the drop-closure-proxies branch June 2, 2017 09:12
@fabpot fabpot mentioned this pull request Jun 5, 2017
fabpot added a commit that referenced this pull request Oct 6, 2017
This PR was merged into the 3.3 branch.

Discussion
----------

[DependencyInjection] remove unused fixtures file

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

I was wondering why the conflict markers in the `master` branch (see https://github.com/symfony/symfony/blob/b6e9471ded49e698c8c2468be9c1b42c621efeac/src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services31.php#L43-L52) did not cause any issues. Turned out that this fixtures file was not used anymore (it was only partially removed in #23022).

Commits
-------

3702e5f remove unused fixtures file
This was referenced Oct 18, 2017
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