Skip to content

Conversation

@damienalexandre
Copy link
Contributor

Goal

Symfony 8 has been released, this PR allow installing the bundle on it.

Changeset

Testing

make is green. I suspect old versions are not going to like the typehint.

* @return void
*/
public function load(array $configs, ContainerBuilder $container)
public function load(array $configs, ContainerBuilder $container): void
Copy link

Choose a reason for hiding this comment

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

Adding return types won't work as long as this bundle still supports PHP 5.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I had to do it because it's mandatory since PHP 8.2 : https://3v4l.org/Pi1eW#vnull and added in the Symfony Interface since Symfony 8.0 (symfony/symfony#60697).

Maybe we can wrap this class in a condition (like this), so we can have two implementation?

WDYT?

EDIT: just tested https://3v4l.org/dZ4OT#v5.6.5 and the parser is still screaming:

Parse error: syntax error, unexpected ':', expecting ';' or '{' in /in/dZ4OT on line 19

So it would needs to be handled by the autoloader somehow.

@agrzegorzewski
Copy link
Contributor

Hi @damienalexandre,
Thanks for bringing this issue to our attention.
We will include your changes as a part of our next release which will add support for Symfony 8. As noted by @derrabus, due to the syntax incompatibility this will result in us dropping support for PHP 5 and releasing a new major version of the notifier.
#188

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants