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

[ZF3] [BC-BREAK] Remove ServiceLocatorAwareInterface from AbstractController #4

Conversation

stefanotorresi
Copy link
Contributor

As discussed in zendframework/zendframework#5168.

Mantaining BC is actually as easy as implementing ServiceLocatorAwareInterface explicitly and importing ServiceLocatorAwareTrait into userland controller classes.

This is just one little step towards zendframework/zendframework#6068.

@macnibblet
Copy link
Contributor

Woho 👍

@gianarb
Copy link

gianarb commented Jun 4, 2015

👍

@weierophinney
Copy link
Member

I hate to be constantly the voice of doom, but this "feature" is a no-starter from a migration stand-point.

Let's consider what a user would do on upgrading:

  • Whoa! Nothing works!
  • What do I need to do?
  • Oh, I have to go into each of my controllers and:
    • Import the ServiceLocatorAwareInterface
    • Import the ServiceLocatorAwareTrait
    • Add a use ServiceLocatorAwareTrait inside my class definition

All that, just to get the application back working the way it was.

IF you provide a tool that can script the above to aid in migration, I'll consider it, as I really, really hate that I capitulated and made controllers service-aware in the first place. But that tool is going to be necessary if we want folks to actually migrate from ZF2 to ZF3. And I really do, because the migration from ZF1 to ZF2 has not happened, precisely because it's very, very hard.

@Ocramius
Copy link
Member

Ocramius commented Jun 4, 2015

PLING!
Suggestion: stop documenting the AbstractController and instead suggest a
no-inheritance approach for our users?

class MyController
{
    public function __construct($helpers) {
        $this->helpers = $helpers;
    }
    public function __invoke($request, $response, $next) {
        // LOGIC, DO YOU SPEAK IT?!
        return $next($request,
$response->withHeader('Location: ' . $this->helpers->url('homepage')));
    }
}

End of service announcement.

Marco Pivetta

http://twitter.com/Ocramius

http://ocramius.github.com/

On 4 June 2015 at 21:06, weierophinney notifications@github.com wrote:

I hate to be constantly the voice of doom, but this "feature" is a
no-starter from a migration stand-point.

Let's consider what a user would do on upgrading:

  • Whoa! Nothing works!
  • What do I need to do?
  • Oh, I have to go into each of my controllers and:
    • Import the ServiceLocatorAwareInterface
    • Import the ServiceLocatorAwareTrait
    • Add a use ServiceLocatorAwareTrait inside my class definition

All that, just to get the application back working the way it was.

IF you provide a tool that can script the above to aid in migration,
I'll consider it, as I really, really hate that I capitulated and made
controllers service-aware in the first place. But that tool is going to be
necessary if we want folks to actually migrate from ZF2 to ZF3. And I
really do, because the migration from ZF1 to ZF2 has not happened,
precisely because it's very, very hard.


Reply to this email directly or view it on GitHub
#4 (comment).

@stefanotorresi
Copy link
Contributor Author

@Ocramius I suppose that's exactly what the new middleware approach will be about, but the existing MVC layer still needs some love.

@weierophinney Fair point; I'll take the burden to provide a migration tool (which will be full of notices of death). Anything to make this happen...

@weierophinney
Copy link
Member

@stefanotorresi Many thanks. :) Migration is a key concern; I'm quite happy with most of the changes, but anything that breaks current apps, I really, really want to provide automated migration tooling whenever possible.

@@ -0,0 +1,16 @@
<?php
/**
Copy link
Member

Choose a reason for hiding this comment

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

wrong file block

@Maks3w
Copy link
Member

Maks3w commented Oct 9, 2015

Supersede by #36

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants