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 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 nicolas-grekas added this to the 3.3 milestone Jun 1, 2017
@nicolas-grekas nicolas-grekas force-pushed the nicolas-grekas:drop-closure-proxies branch from 89636d7 to 42ac3eb Jun 1, 2017
@nicolas-grekas nicolas-grekas force-pushed the nicolas-grekas:drop-closure-proxies branch from 42ac3eb to 57daadb Jun 1, 2017
@fabpot
fabpot approved these changes Jun 1, 2017
@nicolas-grekas
Copy link
Member Author

nicolas-grekas commented Jun 1, 2017

and green :)

@chalasr
chalasr approved these changes Jun 1, 2017
@lyrixx
Copy link
Member

lyrixx commented Jun 2, 2017

👍

@stof
stof approved these changes Jun 2, 2017
@nicolas-grekas nicolas-grekas merged commit 57daadb into symfony:3.3 Jun 2, 2017
3 checks passed
3 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
fabbot.io Your code looks good.
Details
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 nicolas-grekas:drop-closure-proxies branch Jun 2, 2017
@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
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
You can’t perform that action at this time.