netgen / ezpublish-kernel

eZ Publish API and kernel. This is the heart of eZ Publish 5. This is a v2014.11 kernel, maintained for bugfixes by Netgen.
https://netgen.io
Other
8 stars 18 forks source link

EZP-26071: Download asset binary only if we need to create a variation #54

Closed blankse closed 7 years ago

blankse commented 7 years ago

Original Commit: https://github.com/ezsystems/ezpublish-kernel/commit/96f15b1cb893da9dc2b84fac13389d2c8195732f

blankse commented 7 years ago

I added an composer.lock file with twig version 1.33.2. Version 1.34 needs PHPUnit 6. The original repo has a composer.lock too.

emodric commented 7 years ago

@blankse Are you sure about that? 1.x version of Twig should work on PHPUnit 5.x, no matter which version. Only 2.x requires PHP 7.1 and PHPUnit 6.

Also, rather than adding a composer.lock, why don't you limit the Twig constraint in composer.json?

As for other changes, I'm vacationing today, so I'll take a look next week :)

blankse commented 7 years ago

@emodric Yes see here: https://github.com/twigphp/Twig/pull/2452 This is merged in the 1.x branch.

I can limit the twig constraint. I will change it :)

emodric commented 7 years ago

That PR means support for PHPUnit 5.4+ :) Namespaced PHPUnit classes are available since 5.4 :)

blankse commented 7 years ago

OK :) But we have version 4 ;)

emodric commented 7 years ago

Argh! You're right :)

Then Twig constraint is the way to go :) :+1:

emodric commented 7 years ago

Thanks @blankse :)

emodric commented 7 years ago

@blankse It seems there's an issue with limiting Twig to < 1.34. Symfony as of version 2.7.29 requires Twig 1.34 and above, which conflicts with the installation of latest version eZ kernel and the version before (14.12.10) is automatically installed instead.

Is there a way to allow Twig > 1.34?

blankse commented 7 years ago

So the composer.lock for tests was not too bad ;) Otherwise we must backport this when it is finished I think: https://github.com/ezsystems/ezpublish-kernel/pull/2030

emodric commented 7 years ago

For tests, yes, but composer.lock would not be used when installing inside of eZ Publish Community Edition, so you would still get the conflict.

Did you try using 4.8.35 version of PHPUnit which has support for namespaced TestCase? I think that might solve your problem?

Basically, setting phpunit/phpunit to ^4.8.35 ?

blankse commented 7 years ago

We don't need the twig constraint with a composer.lock. So we would not get the conflict. I test the phpunit update: https://github.com/netgen/ezpublish-kernel/pull/55