Removed type hint to enable redirect target to be a content document #56
Conversation
dantleech
force-pushed
the
remove_typehint
branch
from
0ef35fa to
517784a
Compare
|
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? |
| @@ -59,12 +59,12 @@ public function getAutoRouteTag(); | |||
| public function setType($mode); | |||
|
|
|||
| /** | |||
| * For use in the REDIRECT mode, specifies the AutoRoute | |||
| * For use in the REDIRECT mode, specifies the routable object | |||
| * that the AutoRoute should redirect to. | |||
There was a problem hiding this comment.
maybe specify who's job it is to ensure that $autoTarget is a valid target?
|
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. |
|
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. |
|
@wouterj any opinion on this? |
|
ping @wouterj |
|
👍 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). |
|
@dantleech I've added |
Removed type hint to enable redirect target to be a content document
|
Cheers Wouter |
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.