Refactored the switcher extension to get all deps via injection #201
Conversation
| @@ -137,7 +137,9 @@ private function getRequestWithoutLocaleQuery() | |||
| */ | |||
| private function getGuesserMock() | |||
| { | |||
| $mock = $this->getMockBuilder('Lunetics\LocaleBundle\LocaleGuesser\LocaleGuesserInterface')->disableOriginalConstructor()->getMock(); | |||
| $mock = $this->getMockBuilder('Lunetics\LocaleBundle\LocaleGuesser\LocaleGuesserInterface') | |||
There was a problem hiding this comment.
just createMock would be enough, it disables the constructor automatically
| return $this->container->get('lunetics_locale.switcher_helper')->renderSwitch($infos, $template); | ||
| $showCurrentLocale = $this->showCurrentLocaleParam; | ||
| $useController = $this->useControllerParam; | ||
| $allowedLocales = $this->allowedLocalesProvider->getAllowedLocales(); |
There was a problem hiding this comment.
we should inline these assignments into the code below directly, i think
There was a problem hiding this comment.
Yeah indeed. Can I also use short hand array notation yet btw?
Replaced class strings with ::class calls where appropriate Replaced mockBuilder calls with createMock
|
Replaced class strings with |
|
Any update about this? |
|
i hope @lunetics can look at this sometimes. i follow this repository but am not a maintainer of the bundle... |
|
Any news about this one? I am also at rewriting an application and try to fix as many deprecations as possible. Only these two are left in my profiler ... |
|
I think this is a duplicate of #195 that has been merged & released in 2.6.0 |
|
Its not exactly a duplicate. #195 did the minimum to see this bundle run in symfony 4. This one is a proper refactoring to be better at following best practices. It also includes a cleanup of the tests. Its currently in conflict with master and needs a rebase. And it should probably bump the version to 3, there are changes in the constructor of non-final twig extension, so if somebody extended any of the classes or defined their own services with them, things would break. (in version 3, this class should be marked as both Somebody up to rebase this? |
|
Seems fixed in master now |
The main problem that remains is that I don't see any real functional test to check the integration with symfony.
So basically apart from the unit test nothing else broke from the change. Configuration defaults are untested. I can remove an argument from the service definitions and all tests still pass. So that should probably be a new issue.