search

bestit / flagception-bundle

Feature flags on steroids!
MIT License
210 stars 42 forks source link

Symfony 5 fixes #68

Closed hanishsingla closed 4 years ago

hanishsingla commented 4 years ago

Fixes #67

hanishsingla commented 4 years ago

This is failed due to #64 ?

migo315 commented 4 years ago

No this fails because: Declaration of Flagception\Bundle\FlagceptionBundle\Profiler\FeatureDataCollector::collect() must be compatible with Symfony\Component\HttpKernel\DataCollector\DataCollectorInterface::collect(Symfony\Component\HttpFoundation\Request $request, Symfony\Component\HttpFoundation\Response $response, Exception $exception = NULL) in /home/travis/build/bestit/flagception-bundle/src/Profiler/FeatureDataCollector.php

This bundle currently support all symfony versions from 2.7 and above. And the DataCollectorInterface has changed in some versions. For example, in 3.1 it has "Exception" as type for the third argument (https://github.com/symfony/symfony/blob/3.1/src/Symfony/Component/HttpKernel/DataCollector/DataCollectorInterface.php). In 4.3 it hasn't any type (https://github.com/symfony/symfony/blob/4.3/src/Symfony/Component/HttpKernel/DataCollector/DataCollectorInterface.php). And in 5.0 it is throwable now.

But we cannot implement this interface with strict types for all versions. We have to implement it differently for each symfony version. Specially for 4 and 5.

hanishsingla commented 4 years ago

So we can release a new branch (Major version) with the only support of Symfony 5 to keep up to date with new versions of Symfony?

hermann8u commented 4 years ago

Also, this PR is compatible only with Symfony >= 4.3 because the class Symfony\Component\HttpKernel\Event\ControllerEvent only exists since this version.

hanishsingla commented 4 years ago

@hermann8u so it means this bundle can never support Symfony5?

hermann8u commented 4 years ago

Yes it can. But you have to find a workaround to support both event class.

Here is a commit I made on a PR when I had to struggle with the same issue. I hope it can help you to find a solution :

https://github.com/ro0NL/symfony-http-responder/pull/35/commits/679c0e6f38dc83767e79435f23abdb1c77f48bc1

migo315 commented 4 years ago

I‘am currently on vacations. I will support as soon as I am at home (next weekend)

hermann8u commented 4 years ago

IMO, you should consider dropping support for symfony < 3.4 in a new major version and support Symfony 5 in it.

@see https://symfony.com/releases

migo315 commented 4 years ago

Yes, I think that’s the way we have to go.

migo315 commented 4 years ago

I will create a new branch for Symfony 5.0 and upwards today.

NikoGrano commented 4 years ago

@migo315 Could you check this. Its kinda blocking upgrade.

migo315 commented 4 years ago

The master Branch has full Symfony 5 support now. I've done all changes and skip support for Symfony 2+3+4. I've released this changes with bundle Version 4.0.0.

But I also created the branch "3.x", where we can maintain Symfony 2 + 3 + 4.

So, for Symfony 5 use Flagception Bundle 4.0. For Symfony prior 5 use Flagception Bundle 3.0

Let me know if you find any mistakes. My tests were all successful. :-)