asm89 / twig-cache-extension

The missing cache extension for Twig. Caching template fragments with Twig.
MIT License
388 stars 27 forks source link

fix deprecation message. #37

Closed DenysMedvid closed 7 years ago

DenysMedvid commented 7 years ago

Referencing the ... extension by its name (defined by getName()) is deprecated since 1.26 and will be removed in Twig 2.0. Use the Fully Qualified Extension Class Name instead

shieldo commented 7 years ago

As this bundle supports Twig 1.0+, I expect there should be a version check here. It's not always been possible to refer to extensions by their class names.

rvanlaak commented 7 years ago

In my opinion bumping requirements of June 2014 shouldn't be a problem: https://github.com/asm89/twig-cache-extension/blame/master/composer.json#L16

I would advise following the minimum requirements of the Symfony LTS version, which is 2.8 and has php >=5.3.9 and Twig ~1.27 as a minimum: https://github.com/symfony/symfony/blob/2.8/composer.json#L21

shieldo commented 7 years ago

I'd note that the current dependency of the 2.7 branch of Symfony (also a current LTS release) on Twig is at ~1.27, though the last release (2.7.19) was at ~1.26. It's certainly true though to say that Twig compatibility isn't frozen in regard to Symfony, so I wouldn't be against putting it to ~1.26 in a different PR.

From the perspective of this PR, though, it should support any version of Twig that is currently supported.

rvanlaak commented 7 years ago

Great, @DenisMedved we could use Twig_Environment::VERSION for that.

DenysMedvid commented 7 years ago

So, i made some changes. But i do not sure about line. Is there better solution ?

DenysMedvid commented 7 years ago

Or we can change name of extension.

//  Asm89\Twig\CacheExtension\Extension
    public function getName()
    {
        return self::class;
    }
rvanlaak commented 7 years ago

I'd love the FQCN thingy but for that php should be bumped to >=5.4.

And we have version_compare for that?

DenysMedvid commented 7 years ago

@rvanlaak , ok thank you. That line has been replaced to _versioncompare . Looks much better.

asm89 commented 7 years ago

I guess for 1.x support we need 5.3. Let's merge this version for now. Thanks all. :)

msvrtan commented 7 years ago

@asm89 any chance of releasing 1.3.1 to include this fix?

asm89 commented 7 years ago

@msvrtan Done!