DDC-1893 - Updating configuration to reflect latest Doctrine Common changes #396

Merged
merged 6 commits into from Jul 8, 2012

Conversation

Projects
None yet
4 participants
@Ocramius
Member

Ocramius commented Jul 8, 2012

This PR fixes DDC-1893 (http://www.doctrine-project.org/jira/browse/DDC-1893).

The getDefaultAnnotationDriver method was changes to stop supporting older incompatible Doctrine Common versions in favour of the newer logic.

Also, changing logic so that the SimpleAnnotationReader is no more the
default one. An additional parameter for the method will allow using it (this is a BC break!)

The CS fixes that were additionally implemented (along with other minor changes
that do not affect BC compatibility are caused by a CS sniff via IDE.

@beberlei

View changes

lib/Doctrine/ORM/Configuration.php
+ // Register the ORM Annotations in the AnnotationRegistry
+ $reader = new SimpleAnnotationReader();
+ $reader->addNamespace('Doctrine\ORM\Mapping');
+ $cachedReader = new CachedReader($reader, new ArrayCache());

This comment has been minimized.

@beberlei

beberlei Jul 8, 2012

Member

space missing

@beberlei

beberlei Jul 8, 2012

Member

space missing

@beberlei

View changes

lib/Doctrine/ORM/Configuration.php
*/
- public function newDefaultAnnotationDriver($paths = array())
+ public function newDefaultAnnotationDriver($paths = array(), $useDefaultNamespace = false)

This comment has been minimized.

@beberlei

beberlei Jul 8, 2012

Member

i would rename this to $useSimpleAnnotationReader

@beberlei

beberlei Jul 8, 2012

Member

i would rename this to $useSimpleAnnotationReader

+ }
+
+ public static function namedQueryNotFound($queryName)
+ {

This comment has been minimized.

@beberlei

beberlei Jul 8, 2012

Member

where are those coming from?

@beberlei

beberlei Jul 8, 2012

Member

where are those coming from?

This comment has been minimized.

@Ocramius

Ocramius Jul 8, 2012

Member

They're called in ORM\Configuration

@Ocramius

Ocramius Jul 8, 2012

Member

They're called in ORM\Configuration

@beberlei

This comment has been minimized.

Show comment
Hide comment
@beberlei

beberlei Jul 8, 2012

Member

Can you add a line to UPGRADE.md?

Member

beberlei commented Jul 8, 2012

Can you add a line to UPGRADE.md?

+ return new AnnotationDriver(
+ new CachedReader(new AnnotationReader(), new ArrayCache()),
+ (array) $paths
+ );

This comment has been minimized.

@stof

stof Jul 8, 2012

Member

you could make it simpler by sharing the definition of the cachedReader and the driver and using the if only to create the inner reader.

@stof

stof Jul 8, 2012

Member

you could make it simpler by sharing the definition of the cachedReader and the driver and using the if only to create the inner reader.

@stof

This comment has been minimized.

Show comment
Hide comment
@stof

stof Jul 8, 2012

Member

@Ocramius you need to rebase as the Upgrade files have just been concatenated into a single file so the file you edited does not exist anymore

Member

stof commented Jul 8, 2012

@Ocramius you need to rebase as the Upgrade files have just been concatenated into a single file so the file you edited does not exist anymore

@travisbot

This comment has been minimized.

Show comment
Hide comment
@travisbot

travisbot Jul 8, 2012

This pull request fails (merged e0bbad95 into 5adc8be).

This pull request fails (merged e0bbad95 into 5adc8be).

@Ocramius

This comment has been minimized.

Show comment
Hide comment
@Ocramius

Ocramius Jul 8, 2012

Member

@stof still doing other stuff :)

Member

Ocramius commented Jul 8, 2012

@stof still doing other stuff :)

Ocramius added some commits Jul 8, 2012

Updating to reflect latest Doctrine Common changes
Also, changing logic so that the SimpleAnnotationReader is no more the
default one. An additional parameter for the method will allow using it.

The CS fixes that were additionally implemented (along with other minor changes
that do not affect BC compatibility are caused by a CS sniff via IDE.
@travisbot

This comment has been minimized.

Show comment
Hide comment
@travisbot

travisbot Jul 8, 2012

This pull request fails (merged 7c2e5ae into 5adc8be).

This pull request fails (merged 7c2e5ae into 5adc8be).

UPGRADE.md
@@ -1,5 +1,33 @@
# Upgrade to 2.3
+## Configuration *BC Break*
+
+The default annotation syntax has been changed from `@Entity` to `@ORM\Entity`. If you still want to use the simplified

This comment has been minimized.

@stof

stof Jul 8, 2012

Member

you should wrap the lines shorter to be consistent with the other parts of the file

@stof

stof Jul 8, 2012

Member

you should wrap the lines shorter to be consistent with the other parts of the file

@travisbot

This comment has been minimized.

Show comment
Hide comment
@travisbot

travisbot Jul 8, 2012

This pull request fails (merged b67140f into 5adc8be).

This pull request fails (merged b67140f into 5adc8be).

UPGRADE.md
+
+The default annotation syntax has been changed from `@Entity` to `@ORM\Entity`. If you still want to use the simplified
+version, you should use `Doctrine\Common\Annotations\SimpleAnnotationReader` for your AnnotationDriver or call
+`Doctrine\ORM\Configuration#newDefaultAnnotationDriver` with its second parameter set to `true`.

This comment has been minimized.

@beberlei

beberlei Jul 8, 2012

Member

Can you add the parameter to Doctrine\ORM\Tools\Setup class as well?

Also we have to default this parameter to true everywhere i think, because otherwise we have to update the documentation as well.

@beberlei

beberlei Jul 8, 2012

Member

Can you add the parameter to Doctrine\ORM\Tools\Setup class as well?

Also we have to default this parameter to true everywhere i think, because otherwise we have to update the documentation as well.

This comment has been minimized.

@Ocramius

Ocramius Jul 8, 2012

Member

I'm going through the docs looking for what would need update

@Ocramius

Ocramius Jul 8, 2012

Member

I'm going through the docs looking for what would need update

This comment has been minimized.

@stof

stof Jul 8, 2012

Member

all places showing an example with annotations :)

@stof

stof Jul 8, 2012

Member

all places showing an example with annotations :)

This comment has been minimized.

@Ocramius

Ocramius Jul 8, 2012

Member

@stof yeah, @beberlei stopped me, I was going to do that :)

@Ocramius

Ocramius Jul 8, 2012

Member

@stof yeah, @beberlei stopped me, I was going to do that :)

@travisbot

This comment has been minimized.

Show comment
Hide comment
@travisbot

travisbot Jul 8, 2012

This pull request passes (merged 245d906 into 5adc8be).

This pull request passes (merged 245d906 into 5adc8be).

@travisbot

This comment has been minimized.

Show comment
Hide comment
@travisbot

travisbot Jul 8, 2012

This pull request passes (merged 346e34a into 5adc8be).

This pull request passes (merged 346e34a into 5adc8be).

beberlei added a commit that referenced this pull request Jul 8, 2012

Merge pull request #396 from Ocramius/DDC-1893
DDC-1893 - Updating configuration to reflect latest Doctrine Common changes

@beberlei beberlei merged commit 3b04cf5 into doctrine:master Jul 8, 2012

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