add compatibility with Symfony 7.0+ #96
Conversation
|
Thank you for this PR! The version bump could be considered a BC break, can you try to keep compatibility with |
|
Also, tests fail so you probably will need to include |
|
Ok i will try but it seems it's symfony dependencies which are in conflict themselves |
|
@rvanlaak I have check. The problem is due to nyholm/symfony-bundle-test
Maybe it's preferable to remove support of 4.4 version too in futher version of APYBreadcrumbTrailBundle (ex: 2.0.0) otherwise the upgrade to Symfony 7 will be impossible. |
|
I have try to set two vesions of the bundle in composer.json no problems in my repo tests. 🤞 |
🐛 FIX: github actions
|
I have fix all errors from actions. Could you approve again @rvanlaak ? |
| @@ -20,12 +20,12 @@ class Breadcrumb | |||
| /** | |||
| * @var string Title of the breadcrumb | |||
| */ | |||
| private $title = null; | |||
There was a problem hiding this comment.
Is the removals of null a result of the nullable_type_declaration_for_default_null_value PHP CS Fixer change? Not having them set to null could after construction potentially lead to exceptions on read when not being instantiated.
| @@ -16,7 +16,7 @@ | |||
| class Configuration implements ConfigurationInterface | |||
| { | |||
| #[\ReturnTypeWillChange] | |||
| public function getConfigTreeBuilder() | |||
| public function getConfigTreeBuilder(): TreeBuilder | |||
There was a problem hiding this comment.
Is the return type change because of dropping TwigBundle v3.4?
| @@ -19,5 +19,12 @@ | |||
| <tag name="kernel.event_listener" event="kernel.controller" method="onKernelController" priority="-1" /> | |||
| </service> | |||
| <service id="apy_breadcrumb_trail.annotation.listener" alias="APY\BreadcrumbTrailBundle\EventListener\BreadcrumbListener" public="true" /> | |||
|
|
|||
| <service id="Doctrine\Common\Annotations\AnnotationReader" /> | |||
There was a problem hiding this comment.
Doctrine annotations is optional (part of require-dev), so can not be mentioned as service for this bundle, as that will lead to exceptions on non-dev / prod runtime.
Also, as autowiring on this bundle is enabled by default, there should not be a need to define this service from another dependency.
| <service id="Doctrine\Common\Annotations\AnnotationReader" /> | ||
|
|
||
| <service id="APY\BreadcrumbTrailBundle\EventListener\BreadcrumbListener"> | ||
| <argument type="service" id="Doctrine\Common\Annotations\AnnotationReader" /> |
There was a problem hiding this comment.
Can you try getting this done with the autowiring that is enabled on this bundle?
Theses modifications add Symfony 7.0+ compatibility