[WIP] ORM based dynamic routing #122
Conversation
dbu
commented
Jul 19, 2013
dbu
commented
| Q | A |
|---|---|
| Bug fix? | no |
| New feature? | yes |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | TODO |
| Fixed tickets | #95 |
| License | MIT |
| Doc PR | TODO |
|
@matteocaberlotto do you want to pick up from here again? i started porting over the code you did in #95 |
|
yes ofc, the old one has no more reason to exist. i will give a check to tests and other things soon, thx |
|
thanks a lot. this PR is missing the entity class extending the one from |
|
maybe there's no need to add the code to the bundle but only some documentation. i was going to check this too |
| */ | ||
| class RouteProvider extends DoctrineProvider implements RouteProviderInterface | ||
| { | ||
| protected function getCandidates($url) |
There was a problem hiding this comment.
Is this logic the same as with the ODM? Should we encapsulate it in a class?
There was a problem hiding this comment.
no, the phpcr is using id prefix all over the place here
|
hi, i dont know if it's possible/convenient atm but maybe this branch could be rebased and take advantage of last configuration addition to simplify the orm integration. |
|
well, this is going to be close run if we make it into 1.0 :-) but lets try, would be really cool i just rebased. i hope its halfways understandable how we intended the things to work? |
|
u mean the doctrine provider part? |
|
yes, the doctrine orm provider part. its not that much missing... do you have some time to have a look tomorrow? |
|
yes, i just have an error getting the repository, then everything should be ok for a PR against this branch |
|
great! |
|
thx |
| * | ||
| * @author [email protected] | ||
| */ | ||
| class RouteProvider extends DoctrineProvider implements RouteProviderInterface |
There was a problem hiding this comment.
Did you forget a use statement for DoctrineProvider?
There was a problem hiding this comment.
yes, there's some more missing which is going to be submitted in a pull request tonite, then code will be available for review
|
merged the contributions by @matteocaberlotto and did some cleanup. phpcr is still fine, but we lack tests for orm. @dantleech does the cmf testing component already provide something for orm testing? would it be hard to add something? @matteocaberlotto did you not create the RedirectRoute on purpose? i think it would be nice to have it too. |
| @@ -55,7 +85,7 @@ private function buildBasePhpcrCompilerPass() | |||
| return new DoctrinePhpcrMappingsPass( | |||
| $driver, | |||
| array('Symfony\Component\Routing'), | |||
| array('cmf_routing.manager_name'), | |||
| array('cmf_routing.dynamic.persistence.phpcr.manager_name'), | |||
There was a problem hiding this comment.
i guess we need this fix even if we do not merge this PR
|
i would love to merge the PR in. ideally we can make orm provider work properly for 1.0, the worst case is that we ship some dead code. |
|
it would be very good indeed to ship this .. to make our message clearer in regards to persistence. in this spirit i am fine to include this in RC1 .. though if we do then we should commit to getting it running. |
[WIP] ORM based dynamic routing
