search

symfony-cmf / routing-auto

RoutingAuto component
https://cmf.symfony.com
Other
6 stars 11 forks source link

improved DX for suggifier #20

Closed wouterj closed 9 years ago

wouterj commented 9 years ago
Q A
Bug fix? yes
New feature? no
BC breaks? yes
Deprecations? no
Tests pass? yes
Fixed tickets #19
License MIT
Doc PR -

It turns out this wasn't a prophecy issue...

dantleech commented 9 years ago

I wonder if this is the best way to solve this issue. There are only 2 "method" classes, would it make more sense to abstract the common functionality?

wouterj commented 9 years ago

@dantleech that's what I was thinking too. I'll split the second commit into another PR, so we can focus on HHVM compatibility on this PR (which, btw, is now ready)

lsmith77 commented 9 years ago

feel free to also remove the allowed failure on HHVM too as part of this PR

lsmith77 commented 9 years ago

I cherry-picked your first commit into master already and removed the allowed failure

lsmith77 commented 9 years ago

into master and 1.0 btw

wouterj commented 9 years ago

@dantleech I've moved all common functionality for content method providers to an abstract class now.

dantleech commented 9 years ago

Test failing: Declaration of Symfony\Cmf\Component\RoutingAuto\TokenProvider\ContentMethodProvider::normalizeValue() ...

dantleech commented 9 years ago

Why merging into 1.0?

dantleech commented 9 years ago

ping @WouterJ

wouterj commented 9 years ago

Closing in favor of #33