Support FQCN service names for Sensio Framework Extra Bundle #395
Conversation
|
hi, thanks for looking into this! the 1.3 branch is in maintenance mode and does not allow symfony 4, i don't think we want to update this branch to ever support symfony 4. can you please redo this PR against the master branch instead? |
|
Hi @dbu! It's not about Symfony 4, but SensioFrameworkExtraBundle 4.x :) I can redo the patch on the master branch, but then it would be great if 1.3 branch limited sensio extra bundle to 3.x, to disallow installing higher versions. |
There was a problem hiding this comment.
ah, need to read more closely. thanks for the clarification.
i see that in require-dev we limit the extra bundle to 2.3 or 3, but we indeed do not specify that 4 is not compatible. the fix is small enough, lets put it into 1.3 then. if i do a patch version that conflicts with framework extra 4, composer will just install the previous patch version of fos httpcache bundle and we still have the problem.
there is a mistake in the PR, can you please fix that? and can you rebase your branch on 1.3 to get the latest styleci fixes please?
| private function hasControllerListener() | ||
| { | ||
| return $container->has('sensio_framework_extra.controller.listener') || | ||
| $container->has('Sensio\Bundle\FrameworkExtraBundle\EventListener\ControllerListener'); |
There was a problem hiding this comment.
please use \\ to make this strictly correct.
in master we can use the ::class constant, but 1.3 unfortunately still supports php 5.3
|
|
||
| private function hasControllerListener() | ||
| { | ||
| return $container->has('sensio_framework_extra.controller.listener') || |
There was a problem hiding this comment.
you need to pass the $container to this helper method.
|
@dbu Rebased, fixed the Want me to do a PR on |
|
Thanks @dbu ! |
|
tagged https://github.com/FriendsOfSymfony/FOSHttpCacheBundle/releases/tag/1.3.11 for the 1.3 version. merging to master worked without conflicts, git understood that the file is the same and just renamed ;-) |
|
Thanks @emodric , @dbu would we also be seeing these changes in versions 2.x? I had also commented possibly running into this https://github.com/sensiolabs/SensioFrameworkExtraBundle/issues/512 |
|
@The-Don-Himself yes, i merged 1.3 to master to get this in 2.x as well. its here: https://github.com/FriendsOfSymfony/FOSHttpCacheBundle/blob/master/src/DependencyInjection/Compiler/TagListenerPass.php it will be in 2.2, which will be released when symfony 3.4 is out. |
From version 4.0 and up,
ControllerListenerservice fromSensioFrameworkExtraBundlehas been renamed to use FQCN as service name, so this adds support for the old and new name.masterbranch also needs this fix, however, the compiler pass has been renamed toTagListenerPass, so probably needs manual merge.