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

Fix EntityManager namespace #5666

Closed
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

@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

@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
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
@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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants