search

swarrot / SwarrotBundle

A symfony bundle for swarrot integration
MIT License
89 stars 59 forks source link

PHP 7.1? #132

Closed zspine closed 2 years ago

zspine commented 6 years ago

php-version: 7.0.10 symfony-version: 3.4 swarrot-bundle: 1.5.0 (current version)

Implementing "Symfony\Component\HttpKernel\DataCollector\DataCollectorInterface" without the "reset()" method is deprecated since Symfony 3.4 and will be unsupported in 4.0 for class "Swarrot\SwarrotBundle\DataCollector\SwarrotDataCollector".

Hi,

Above deprecation triggered on symfony version 3.4 and problem already seems to be fixed by the pull request but unfortunately it is not possible to upgrade to latest version of swarrot-bundle from php 7.0.26.

swarrot/swarrot-bundle v1.5.1 requires php >=7.1 -> your PHP version (7.0.10) does not satisfy that requirement.

I am also confused with a breaking change from 1.5.0 to 1.5.1, any clean work around without modifying the file manually?

Thank you

odolbeau commented 6 years ago

Hi @zspine

Given the difficulty to maintain branches for several PHP / Symfony version, we only have one master branch with several tags. In this case, this is problematic as this patch should be available for PHP 7.0 versions (not only for 7.1+).

Unfortunately, I don't have any clean solution to fix this problem for PHP 7.0. As this version of PHP is not actively supported anymore, we dropped its support from swarrot. You an either ignore this deprecation notice or override this DataCollector class in your project. That's far from perfect but I'm afraid I don't see any other solution right now. :/

Given the breaking change your talking about between 1.5.0 & 1.5.1, what is it? Bumping the minimal PHP version isn't a breaking change.

stof commented 6 years ago

@odolbeau dropping support for PHP versions should at least have been done in a minor version IMO (allowing us to create a new patch release for the previous minor if necessary)

odolbeau commented 6 years ago

Is it too late to correct this? It's easy to release a 1.6.0 version from master but I'm not sure it's a good idea to release a 1.5.2 version which is 1.5.1 version without commit ac545fa which remove php7.0 support.

WDYT?

zspine commented 6 years ago

Hi @odolbeau ,

:( Thank you for the detailed explanation. I think I have misunderstood the breaking change concept, it make sense now. I have temporarily modified the file to avoid logging these exceptions and it works fine. But it would be great if there is a way to fix this properly as @stof suggested.

cheers!

stof commented 2 years ago

Closing this as it is too late to change that anyway now