8000 [DependencyInjection] fixed regression where setting a service to null did not trigger a re-creation of the service when getting it (closes #8392) by fabpot · Pull Request #8582 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[DependencyInjection] fixed regression where setting a service to null did not trigger a re-creation of the service when getting it (closes #8392) #8582

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 25, 2013
Merged

[DependencyInjection] fixed regression where setting a service to null did not trigger a re-creation of the service when getting it (closes #8392) #8582

merged 1 commit into from
Jul 25, 2013

Conversation

fabpot
Copy link
Member
@fabpot fabpot commented Jul 25, 2013
Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #8392
License MIT
Doc PR n/a

@fabpot fabpot mentioned this pull request Jul 25, 2013
@stof
Copy link
Member
stof commented Jul 25, 2013

you should add a test to avoid another regression

@fabpot
Copy link
Member Author
fabpot commented Jul 25, 2013

@stof I'm waiting for Travis first ;)

@fabpot
[DependencyInjection] fixed regression where setting a service to nul…
50d0727 
…l did not trigger a re-creation of the service when getting it
fabpot added a commit that referenced this pull request Jul 25, 2013
@fabpot
merged branch fabpot/null-services (PR #8582)
7dc211a 
This PR was merged into the 2.3 branch.

Discussion
----------

[DependencyInjection] fixed regression where setting a service to null did not trigger a re-creation of the service when getting it (closes #8392)

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #8392
| License       | MIT
| Doc PR        | n/a

Commits
-------

50d0727 [DependencyInjection] fixed regression where setting a service to null did not trigger a re-creation of the service when getting it
@fabpot fabpot merged commit 50d0727 into symfony:2.3 Jul 25, 2013
@fabpot fabpot deleted the null-services branch April 27, 2014 09:25
@ro0NL ro0NL mentioned this pull request Aug 21, 2016
Closed
fabpot added a commit that referenced this pull request Aug 22, 2016
@fabpot
minor #19689 [DI] Cleanup array_key_exists (ro0NL)
c89f80a 
This PR was squashed before being merged into the 2.7 branch (closes #19689).

Discussion
----------

[DI] Cleanup array_key_exists

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | comma-separated list of tickets fixed by the PR, if any
| License       | MIT
| Doc PR        | reference to the documentation PR, if any

Investigated this a bit, and to me it looks like left-over code. `null` doesnt end up in `$this->services` by design (this was done in #8582) so it seems. The test added then for regression still passes :)

I cant believe we guarantee BC for users doing `$this->services['id'] = null` (due protected), allowing them to break the design of `has`, `get` and `initialized` right now.

This also happens for `$this->definitions` in `ContainerBuilder`, maybe because `Container` did it alteady.. but im not sure.

Then again, there's this comment: #14470 (comment)

Commits
-------

3306c70 [DI] Cleanup array_key_exists
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@fabpot @stof
0