Skip to content
This repository has been archived by the owner on Jan 29, 2020. It is now read-only.

[ZF3] [bc-break] remove ControllerLoader, use ControllerManager instead #6

Closed
wants to merge 5 commits into from
Closed

[ZF3] [bc-break] remove ControllerLoader, use ControllerManager instead #6

wants to merge 5 commits into from

Conversation

samsonasik
Copy link
Contributor

This was referenced Jun 4, 2015
Closed
Closed
@Ocramius Ocramius added this to the 3.0.0 milestone Jun 4, 2015
@samsonasik
Copy link
Contributor Author

rebased

Maks3w
Maks3w reviewed Jun 6, 2015
View reviewed changes
test/Service/ControllerManagerFactoryTest.php
$this->manager = $this->services->get('ControllerManager');
$config = array(
'invokables' => array(
>>>>>>> remove ControllerLoader, use ControllerManager instead:test/Service/ControllerManagerFactoryTest.php
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bad rebase

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed.

Maks3w
Maks3w reviewed Jun 6, 2015
View reviewed changes
src/Service/ControllerManagerFactory.php
@@ -13,7 +13,7 @@
use Zend\ServiceManager\FactoryInterface;
use Zend\ServiceManager\ServiceLocatorInterface;

class ControllerLoaderFactory implements FactoryInterface
class ControllerManagerFactory implements FactoryInterface
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead create a new class and inherit the old one from the new one. By this way BC is preserved

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as this is for ZF3, I thik it is ok to remove the old one /cc @weierophinney

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do it and this could be merged in 2.6

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done, and ControllerLoaderFactory marked as @deprecated.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as zendframework/zend-authentication#2 (comment) , ControllerLoader actually removed for 3.0

Maks3w
Maks3w reviewed Jun 6, 2015
View reviewed changes
src/Service/ServiceListenerFactory.php
@@ -44,7 +44,7 @@ class ServiceListenerFactory implements FactoryInterface
'factories' => [
'Application' => 'Zend\Mvc\Service\ApplicationFactory',
'Config' => 'Zend\Mvc\Service\ConfigFactory',
'ControllerLoader' => 'Zend\Mvc\Service\ControllerLoaderFactory',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Old key must be preserved for bc

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a comment with @deprecated for to remove in the future

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

now it for 3.0, can be actually removed.

@Maks3w Maks3w removed this from the 3.0.0 milestone Jun 6, 2015
@Maks3w Maks3w removed the BC Break label Jun 6, 2015
Maks3w
Maks3w reviewed Jun 6, 2015
View reviewed changes
src/Service/ControllerLoaderFactory.php
{
/**
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a constructor with

trigger_error('Description', E_USER_DEPRECATED);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@samsonasik samsonasik changed the title remove ControllerLoader, use ControllerManager instead deprecate ControllerLoader, suggest to use ControllerManager instead Jun 7, 2015
@samsonasik
Copy link
Contributor Author

I've updated PR title

@samsonasik
Copy link
Contributor Author

based on @weierophinney comment on zendframework/zend-authentication#2 (comment) , I will update it as fully remove ControllerLoader, and it will be a PR for 3.0

@samsonasik samsonasik changed the title deprecate ControllerLoader, suggest to use ControllerManager instead [ZF3] [bc-break] remove ControllerLoader, use ControllerManager instead Jun 8, 2015
@samsonasik
Copy link
Contributor Author

PR title and commits force updated so we use it for 3.0

@samsonasik
Copy link
Contributor Author

@Ocramius it can be marked as 3.0.0 milestone again ;)

@Ocramius Ocramius added this to the 3.0.0 milestone Jun 8, 2015
@Maks3w
Copy link
Member

Maks3w commented Jul 22, 2015

With the previous approach this could be merged right now. With the v3 tunel vision things will be freeze sine die

@Maks3w
Copy link
Member

Maks3w commented Oct 9, 2015

Supersede by #36

@Maks3w Maks3w closed this Oct 9, 2015
@mushroom11s mushroom11s mentioned this pull request Jan 2, 2019
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
BC Break enhancement
Projects
None yet
Milestone
3.0.0
Development

Successfully merging this pull request may close these issues.
3 participants
@samsonasik @Maks3w @Ocramius