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

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

Closed
samsonasik wants to merge 5 commits into
zendframework:developfrom
samsonasik:rem-controllerloader
Closed

[ZF3] [bc-break] remove ControllerLoader, use ControllerManager instead#6
samsonasik wants to merge 5 commits into
zendframework:developfrom
samsonasik:rem-controllerloader

Conversation

@samsonasik
Copy link
Copy Markdown
Contributor

@samsonasik samsonasik commented Jun 4, 2015

This was referenced Jun 4, 2015
Closed
Closed
@Ocramius Ocramius added this to the 3.0.0 milestone Jun 4, 2015
@samsonasik samsonasik force-pushed the rem-controllerloader branch from 735a68c to 9d78688 Compare June 6, 2015 00:13
@samsonasik
Copy link
Copy Markdown
Contributor Author

samsonasik commented Jun 6, 2015

rebased

Maks3w
Maks3w reviewed Jun 6, 2015
View reviewed changes
Comment thread test/Service/ControllerManagerFactoryTest.php Outdated
Copy link
Copy Markdown
Member

@Maks3w Maks3w Jun 6, 2015

Choose a reason for hiding this comment

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

Bad rebase

Copy link
Copy Markdown
Contributor Author

@samsonasik samsonasik Jun 6, 2015

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
Comment thread src/Service/ControllerManagerFactory.php
Copy link
Copy Markdown
Member

@Maks3w Maks3w Jun 6, 2015

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
Copy Markdown
Contributor Author

@samsonasik samsonasik Jun 6, 2015

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
Copy Markdown
Member

@Maks3w Maks3w Jun 6, 2015

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
Copy Markdown
Contributor Author

@samsonasik samsonasik Jun 6, 2015

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
Copy Markdown
Contributor Author

@samsonasik samsonasik Jun 8, 2015

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

@samsonasik samsonasik force-pushed the rem-controllerloader branch from 7aede3b to 3bae995 Compare June 6, 2015 10:30
@samsonasik samsonasik force-pushed the rem-controllerloader branch from b4c1a29 to 5e0828c Compare June 6, 2015 10:32
Maks3w
Maks3w reviewed Jun 6, 2015
View reviewed changes
Comment thread src/Service/ServiceListenerFactory.php
Copy link
Copy Markdown
Member

@Maks3w Maks3w Jun 6, 2015

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
Copy Markdown
Member

@Maks3w Maks3w Jun 6, 2015

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
Copy Markdown
Contributor Author

@samsonasik samsonasik Jun 6, 2015

Choose a reason for hiding this comment

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

done

Copy link
Copy Markdown
Contributor Author

@samsonasik samsonasik Jun 8, 2015

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
Comment thread src/Service/ControllerLoaderFactory.php Outdated
Copy link
Copy Markdown
Member

@Maks3w Maks3w Jun 6, 2015

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
Copy Markdown
Contributor Author

@samsonasik samsonasik Jun 6, 2015

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
Copy Markdown
Contributor Author

samsonasik commented Jun 7, 2015

I've updated PR title

@samsonasik
Copy link
Copy Markdown
Contributor Author

samsonasik commented Jun 8, 2015

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 force-pushed the rem-controllerloader branch from 60fa2fb to 73c94e6 Compare June 8, 2015 16:23
@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
Copy Markdown
Contributor Author

samsonasik commented Jun 8, 2015

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

@samsonasik
Copy link
Copy Markdown
Contributor Author

samsonasik commented Jun 8, 2015

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

@Ocramius Ocramius added this to the 3.0.0 milestone Jun 8, 2015
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