Skip to content

[WIP] DDC-1852 - Validating lifecycle callbacks in tools #361

Closed
wants to merge 8 commits into from

3 participants

@Ocramius
Doctrine member
Ocramius commented Jun 4, 2012

This feature simply adds validation for lifecycle callbacks.

This PR introduces some BC Breaks:

  1. Doctrine\ORM\Mapping\ClassMetadataInfo#validateLifecycleCallbacks() has been dropped
  2. Doctrine\ORM\Mapping\HasLifecycleCallbacks has been deprecated as it is not checked anymore. This makes the AnnotationDriver slower at first run, but I think this is where caching solves the problem correctly
  3. Mappings have to be validated via tools, as runtime validation is incomplete anyway. Runtime validation of lifecycle callbacks is dropped.

Build Status

@travisbot

This pull request passes (merged 0cd962d into 9445502).

@Ocramius
Doctrine member
Ocramius commented Jun 6, 2012

Weird that the branch alone fails...

@travisbot

This pull request passes (merged 403aae6 into 9445502).

@Ocramius Ocramius Adding @deprecated to the @HasLifecycleCallbacks annotation a9922e4
@travisbot

This pull request passes (merged a9922e4 into 9445502).

@travisbot

This pull request passes (merged 33f832f into 9445502).

@travisbot

This pull request passes (merged 49c94551 into 9445502).

@travisbot

This pull request passes (merged a8e213bd into 7b75849).

@travisbot

This pull request passes (merged c7d0a99 into 7b75849).

@Ocramius
Doctrine member

@beberlei can't think of any other addition to this. See if you like this idea :)

@beberlei beberlei commented on the diff Jun 18, 2012
lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php
@@ -382,7 +382,6 @@ protected function validateRuntimeMetadata($class, $parent)
$class->validateIdentifier();
$class->validateAssocations();
- $class->validateLifecycleCallbacks($this->getReflectionService());
@beberlei
Doctrine member
beberlei added a note Jun 18, 2012

Why remove this? I think it makes sense here.

@Ocramius
Doctrine member
Ocramius added a note Jun 18, 2012

Since validation has been moved to tools, this shouldn't be needed anymore here, plus it is a check that only worked on XML/YAML/PHP drivers.

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

As discussed on IRC, will (re-)add improved validation support in the ClassMetadataFactory :)

@beberlei
Doctrine member
beberlei commented Jul 4, 2012

Close then?

@Ocramius
Doctrine member
Ocramius commented Jul 4, 2012

Sorry, missed the message. Closing for now :)

@Ocramius Ocramius closed this Jul 4, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.