Need PR review for #4962 ( added "ControllerManager" Manager, and make "ControllerLoader" as alias of it )

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

Need PR review for #4962 ( added "ControllerManager" Manager, and make "ControllerLoader" as alias of it )

samsonasik
This post has NOT been accepted by the mailing list yet.
Hello...
I make this post for review of my latest PR about "added "ControllerManager" Manager, and make "ControllerLoader" as alias of it" for naming consistency at https://github.com/zendframework/zf2/pull/4962 as Matthew suggestion.
I think it's not BC break because of I made new Factory named ControllerManagerFactory and point ControllerLoader as alias of ControllerManager. This is the summary of my PR :
1. create ControllerManagerFactory and make ControllerLoaderFactory as __deprecated__ that extends ControllerManagerFactory, so ControllerLoaderFactory still can be used
2. register service "ControllerManager" for ControllerManagerFactory, and make ControllerLoader as alias for it, so 'ControllerLoader' manager still can be used to avoid BC break.
3. replace usage of "ControllerLoader" to "ControllerManager", and keep test case for ControllerLoader.

Please give feedback, thanks
Reply | Threaded
Open this post in threaded view
|

Re: Need PR review for #4962 ( added "ControllerManager" Manager, and make "ControllerLoader" as alias of it )

Marco Pivetta
This post has NOT been accepted by the mailing list yet.
Looks good to me! I just added my comments on the pull request. I'm just not sure about the ETA for the removal of the previous service



On 20 August 2013 18:20, samsonasik [via Zend Framework Community] <[hidden email]> wrote:
Hello...
I make this post for review of my latest PR about "added "ControllerManager" Manager, and make "ControllerLoader" as alias of it" for naming consistency at https://github.com/zendframework/zf2/pull/4962 as Matthew suggestion.
I think it's not BC break because of I made new Factory named ControllerManagerFactory and point ControllerLoader as alias of ControllerManager. This is the summary of my PR :
1. create ControllerManagerFactory and make ControllerLoaderFactory as __deprecated__ that extends ControllerManagerFactory, so ControllerLoaderFactory still can be used
2. register service "ControllerManager" for ControllerManagerFactory, and make ControllerLoader as alias for it, so 'ControllerLoader' manager still can be used to avoid BC break.
3. replace usage of "ControllerLoader" to "ControllerManager", and keep test case for ControllerLoader.

Please give feedback, thanks


To start a new topic under ZF Contributor, email [hidden email]
To unsubscribe from ZF Contributor, click here.
NAML

Reply | Threaded
Open this post in threaded view
|

Re: Need PR review for #4962 ( added "ControllerManager" Manager, and make "ControllerLoader" as alias of it )

samsonasik
This post has NOT been accepted by the mailing list yet.
Thanks Marco!