Skip to content

[DependencyInjection] resolve circular reference#14446

Merged
nicolas-grekas merged 1 commit into
symfony:2.6from
xabbuh:container-self-reference-merge-fix
Apr 27, 2015
Merged

[DependencyInjection] resolve circular reference#14446
nicolas-grekas merged 1 commit into
symfony:2.6from
xabbuh:container-self-reference-merge-fix

Conversation

@xabbuh

@xabbuh xabbuh commented Apr 22, 2015

Copy link
Copy Markdown
Member
Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #11422, #14445
License MIT
Doc PR

This issue has been fixed in #11422. Due to a bad merge in 3bed1b7 it partially appeared again in Symfony 2.6 or higher.

This issue has been fixed in symfony#11422. Due to a bad merge in
3bed1b7 it partially appeared again in
Symfony 2.6 or higher.
@hacfi

hacfi commented Apr 22, 2015

Copy link
Copy Markdown
Contributor

👍

1 similar comment
@jakzal

jakzal commented Apr 22, 2015

Copy link
Copy Markdown
Contributor

👍

@dosten

dosten commented Apr 22, 2015

Copy link
Copy Markdown
Contributor

The PhpDumper generate the container with the $this->set('service_container', $this) line. see https://github.com/symfony/symfony/blob/2.6/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php#L902
can this be removed and the tests fixed?

@hacfi

hacfi commented Apr 22, 2015

Copy link
Copy Markdown
Contributor

@xabbuh Have a look at https://github.com/symfony/symfony/pull/14445/files to see which changes to the tests are necessary.

@dosten

dosten commented Apr 22, 2015

Copy link
Copy Markdown
Contributor

@hacfi any reason to keep a blank line between $this->scopeStacks = array(); and $this->scopes = array(); in the dumped container?

@xabbuh

xabbuh commented Apr 22, 2015

Copy link
Copy Markdown
Member Author

@dosten @hacfi Imho the change to the PhpDumper then should be done for the 2.3 branch too. What do you think?

@dosten

dosten commented Apr 22, 2015

Copy link
Copy Markdown
Contributor

@xabbuh 👍

@hacfi

hacfi commented Apr 22, 2015

Copy link
Copy Markdown
Contributor

@dosten Nope :)..just didn’t think about that.

@nicolas-grekas

Copy link
Copy Markdown
Member

👍

@nicolas-grekas

Copy link
Copy Markdown
Member

Thank you @xabbuh.

@nicolas-grekas nicolas-grekas merged commit 8b3b3ce into symfony:2.6 Apr 27, 2015
nicolas-grekas added a commit that referenced this pull request Apr 27, 2015
This PR was merged into the 2.6 branch.

Discussion
----------

[DependencyInjection] resolve circular reference

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #11422, #14445
| License       | MIT
| Doc PR        |

This issue has been fixed in #11422. Due to a bad merge in 3bed1b7 it partially appeared again in Symfony 2.6 or higher.

Commits
-------

8b3b3ce [DependencyInjection] resolve circular reference
nicolas-grekas added a commit that referenced this pull request Apr 27, 2015
This PR was merged into the 2.3 branch.

Discussion
----------

[DependencyInjection] Fixed missing tests

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

Follow up of #11422 and #14446

Commits
-------

2892902 Fixed tests
@xabbuh xabbuh deleted the container-self-reference-merge-fix branch April 27, 2015 21:07
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.

5 participants