Minishlink / web-push-bundle

Bundle around the WebPush library
MIT License
45 stars 10 forks source link

Support Minishlink/web-push version 5 #12

Closed arjenm closed 4 years ago

arjenm commented 5 years ago

This bundle is limited to version 4 of minishlink/web-push, but version 5 is here for some time now. Please support it in a new major version of the.

And perhaps just skip version 4 in that bundle to synchronize the major version of the bundle with the main library :)

It appears there is one deprecation which pops up with 4.2

https://github.com/Minishlink/web-push-bundle/blob/0c3b1fe4156dee7491bc75cdfbbe25c1fd74c0a6/DependencyInjection/Configuration.php#L17

        $treeBuilder = new TreeBuilder();
        $rootNode = $treeBuilder->root('minishlink_web_push');
// Should become
        $treeBuilder = new TreeBuilder('minishlink_web_push');
        $rootNode = $treeBuilder->getRootNode();
juliusstoerrle commented 4 years ago

the deprecation warning would fixed by the update to symfony 5 in #14

Minishlink commented 4 years ago

Fixed in v4.0.0, thanks guys!

juliusstoerrle commented 4 years ago

I did not update the minishlink/web-push version to V5 only symfony. So this is still open. I will create a pull request for this. There are two options either allow ^5.0 or ^4.0.0|^5.0, not sure what you prefer?

arjenm commented 4 years ago

The bundle should work with the latest version of what it bundles. But you're right that can be done with still supporting v4.

However, there were some BC-changes with minishlink/web-push v5, so perhaps its more correct to create a v5 of this bundle that only supports v5 of minishlink/web-push. Than again, that may be confusing since v5 of Symfony was also just released :P