symfony-cmf / routing-auto

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

Removed type hint to enable redirect target to be a content document #56

Closed dantleech closed 9 years ago

dantleech commented 9 years ago

Fix for #43

Currently we only support creating a redirect to another route. This means that if a route is renamed 50 times and somebody visits the initial route, a chain of 50 route documents will need to be resolved.

With this change it will be possible for the adapter to assign the content as the redirect target.

dbu commented 9 years ago

i fear that this is a BC break for anybody implementing the interface, as the implementation may not narrow down what is allowed. but it does make a lot of sense to do it like this. so maybe get the bundle ready and declare this lib conflicts with the bundle < 1.1?

dantleech commented 9 years ago

Yeah, it certainly is a BC break and we would be bad to do it .. could also be seen as a BC breaking bug fix. Alternatively we could just introduce a new method and deprecate the current one, but it already has the best name.

dbu commented 9 years ago

i would be ok with considering it a BC breaking bugfix if we do a big note in the CHANGELOG and make sure to conflict with the routing-auto-bundle < 1.1 in composer.json to avoid a mess.

dantleech commented 9 years ago

@WouterJ any opinion on this?

dantleech commented 9 years ago

ping @WouterJ

wouterj commented 9 years ago

:+1: to see this as a BC breaking bug fix, but please add a more clear BC break warning in the CHANGELOG (creating an UPGRADE-1.1.md file seems overkill here).

wouterj commented 9 years ago

@dantleech I've added [BC Break] to the changelog, feel free to merge.

dantleech commented 9 years ago

Cheers Wouter