Skip to content

Fix EntityManager namespace #5666

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
Closed
wants to merge 2 commits into from
Closed

Fix EntityManager namespace #5666

wants to merge 2 commits into from

Conversation

@JhonnyL
Copy link
Contributor

@JhonnyL JhonnyL commented Sep 2, 2015

Q A
Doc fix? yes
New docs? no
Applies to all
Fixed tickets -
@javiereguiluz
Copy link
Member

javiereguiluz commented Sep 2, 2015

@JhonnyL thanks for reporting this issue. Although your solution works, this other solution is also possible:

use Doctrine\Common\Persistence\ObjectManager;

And of course, change the typehint from EntityManager to ObjectManager in the method. What do you think?
@stof
Copy link
Member

stof commented Sep 2, 2015

and I think the current buggy code comes from a failed attempt at migrating from the ORM-specific typehint to the common interface.
@JhonnyL
Copy link
Contributor Author

JhonnyL commented Sep 2, 2015

It was changed from ObjectManager to EntityManager in #5456.
@JhonnyL
Copy link
Contributor Author

JhonnyL commented Sep 2, 2015

@javiereguiluz @stof It was changed to EntityManger in a previous commit cause of inconsistency with the variable name.

Typehinting the ObjectManger is probably better, but then I should also change the variable name to $manager. What do you think?
@javiereguiluz
Copy link
Member

javiereguiluz commented Sep 2, 2015

@JhonnyL to me it makes sense to rename it to $manager
@xabbuh
Copy link
Member

xabbuh commented Sep 5, 2015

👍
@wouterj
Copy link
Member

wouterj commented Sep 5, 2015

Thanks, it looks perfect now!
wouterj added a commit that referenced this pull request Sep 5, 2015
@wouterj
bug #5666 Fix EntityManager namespace (JhonnyL)
3c0036e 
This PR was squashed before being merged into the 2.3 branch (closes #5666). Discussion ---------- Fix EntityManager namespace | Q | A | ------------- | --- | Doc fix? | yes | New docs? | no | Applies to | all | Fixed tickets | - Commits ------- a8b2d6d Fix EntityManager namespace
@wouterj wouterj closed this Sep 5, 2015
@wouterj wouterj mentioned this pull request Sep 5, 2015
Merged
@JhonnyL JhonnyL deleted the fix-entitymanager-namespace branch September 7, 2015 08:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Status: Reviewed

5 participants

@JhonnyL @javiereguiluz @stof @xabbuh @wouterj