Fix access level violation #17

Merged
merged 1 commit into from Sep 26, 2012

Conversation

Projects
None yet
2 participants
Contributor

saem commented Sep 25, 2012

The getLoader interface has changed and this causes immediate breakage (even upon something simple like php app/console).

I've updated it here, fixed up the doc comment (now correctly indicates that this will immediately throw an exception).

@stof stof and 1 other commented on an outdated diff Sep 25, 2012

Form/Type/DocumentType.php
*
- * @param ObjectManager $manager
- * @param array $options
- * @return EntityLoaderInterface
+ * @param \Doctrine\Common\Persistence\ObjectManager $manager
@stof

stof Sep 25, 2012

Member

Please use the short class name in the phpdoc here (as done previously)

@saem

saem Sep 25, 2012

Contributor

I used PHPStorm, and it automatically generated that -- sucks because it didn't notice the use statements. I imagine this would also effect code completion for other IDEs, but I can short name the ObjectManager, and the FormException, but I have to put in a use stateument to shorten the EntityLoaderInterface for it to be resolve correctly.

I'm not sure what the style guide is here, should I short name everything, and put in the appropriate use statements? Or only shorten the items that have use statements and remain resolvable?

@stof

stof Sep 25, 2012

Member

@saem In Symfony2, @fabpot prefers keeping the short name everywhere and adding the needed use statement for this. I don't know what @beberlei prefers for this bundle though. But in any case, it should be resolvable (which was indeed not the case here previously)

@stof

stof Sep 25, 2012

Member

And btw, PhpStorm 5 handles use statements properly when resolving the phpdoc, even if it generates it with the FQCN.

@saem saem Fix access level violation
The getLoader interface has changed and this causes immediate breakage (even upon something simple like php app/console).

I've updated it here, fixed up the doc comment (now correctly indicates that this will immediately throw an exception).
ce6ad87
Contributor

saem commented Sep 25, 2012

Redone, using all short names.

Member

stof commented Sep 26, 2012

Actually, I have merge rights on this repo so let's merge :)

stof merged commit 1967e38 into doctrine:master Sep 26, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment