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

Added the support of namespace alias in getManagerForClass #166

Merged
merged 1 commit into from
Jul 23, 2012

Conversation

stof
Copy link
Member

@stof stof commented Jul 18, 2012

See doctrine/DoctrineBundle#95

@beberlei it would be great to have it in the 2.3 branch at least, otherwise I will have to overwrite the method in the Symfony bridge to avoid a regression due to the change in the DoctrineType.

@schmittjoh
Copy link
Member

Is there a test for this?

@stof
Copy link
Member Author

stof commented Jul 18, 2012

There is no test at all for the abstract registry

@stof
Copy link
Member Author

stof commented Jul 18, 2012

Btw, the logic is the same than in the AbstractClassMetadataFactory

@@ -190,6 +190,12 @@ public function getManager($name = null)
*/
public function getManagerForClass($class)
{
// Check for namespace alias
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stof the abstract class metadata factory already handles this (see https://github.com/doctrine/common/blob/master/lib/Doctrine/Common/Persistence/Mapping/AbstractClassMetadataFactory.php#L346 )

So this and the block can simply be removed. Not sure about the one using ReflectionClass, since isTransitient is quite simple.
Also, if you call $this->getAliasNamespace() you have at least to define it as abstract method.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no it cannot be removed. See the line just after this creating a ReflectionClass and the issue linked in the description.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and I don't need to define the method as abstract: the interface already does it

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This happens when I'm too lazy to browse :)

@schmittjoh
Copy link
Member

Maybe you can still add a test (even if we do not have one on this yet).

@guilhermeblanco
Copy link
Member

+1

4 similar comments
@linniksa
Copy link

+1

@icedmaster
Copy link

+1

@vmkudrin
Copy link

+1

@lobster-tm
Copy link

+1

beberlei added a commit that referenced this pull request Jul 23, 2012
Added the support of namespace alias in getManagerForClass
@beberlei beberlei merged commit 8cb70d8 into doctrine:master Jul 23, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants