Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

[DependencyInjection] resolve circular reference #14446

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

Merged

Conversation

xabbuh
Copy link
Member

@xabbuh xabbuh commented Apr 22, 2015

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
Copy link
Contributor

hacfi commented Apr 22, 2015

👍

1 similar comment
@jakzal
Copy link
Contributor

jakzal commented Apr 22, 2015

👍

@dosten
Copy link
Contributor

dosten commented Apr 22, 2015

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
Copy link
Contributor

hacfi commented Apr 22, 2015

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

@dosten
Copy link
Contributor

dosten commented Apr 22, 2015

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

@xabbuh
Copy link
Member Author

xabbuh commented Apr 22, 2015

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

@dosten
Copy link
Contributor

dosten commented Apr 22, 2015

@xabbuh 👍

@hacfi
Copy link
Contributor

hacfi commented Apr 22, 2015

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

@nicolas-grekas
Copy link
Member

👍

@nicolas-grekas
Copy link
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
Morty Proxy This is a proxified and sanitized view of the page, visit original site.