Fixed deprecation with Symfony 4.2

FriendsOfSymfony / FOSCKEditorBundle

Provides a CKEditor integration for your Symfony project.

Other

518 stars 83 forks source link

Fixed deprecation with Symfony 4.2 #164

Closed jmsche closed 5 years ago

jmsche commented 5 years ago

Q	A
Bug fix?	no
New feature?	no
BC breaks?	no
Deprecations?	no
Fixed tickets	closes #167
License	MIT

Hi, this PR fixes a small deprecation introduced in Symfony 4.2; the fix includes a BC layer for older versions.

kunicmarko20 commented 5 years ago

Had to do the same thing, thank you! Can you maybe check the build errors?

jmsche commented 5 years ago

I'm no Travis expert, but I tried to fix the DI component to version 4.2 for the phpstan check - seems it breaks memory limit (which is set to 1.5GB).

Should I increase composer's memory limit for this build? Do you think this issue is "normal"?

kunicmarko20 commented 5 years ago

Not a normal issue, will check it out tonight

jmsche commented 5 years ago

Just pushed a commit increasing the memory limit of composer - for this check - reveals the sensio/distribution-bundle dependency can't be installed as it requires DI < 4.

Also, an other failing test, but I think it is unrelated to what I did.

Any idea how I can fix this dependency issue?

kunicmarko20 commented 5 years ago

Accidentally closed, didn't have time to check this and now I am afk for a few days. Do we event need distribution bundle?

About test, took a quick look and it seems that new version of phpunit has a different signature which is a BC break, not sure tho

jmsche commented 5 years ago

Do we event need distribution bundle?

For Symfony < 4 I guess. It shouldn't install if Flex is already there.

kunicmarko20 commented 5 years ago

Do we use it in code? If no, drop it, sf should install it as its dependency

kunicmarko20 commented 5 years ago

Update symfony phpunit bridge to 4.1 that should solve it IIRC.

Also, do we still need memory limit?

jmsche commented 5 years ago

I removed composer memory_limit env var as it's not required anymore - needed it to get the error message from composer.

Well, it only gets worse :p I changed the min requirement of phpunit bridge to ^4.1, now two tests fail.

angelsk commented 5 years ago

Try adding - SYMFONY_PHPUNIT_VERSION="6.5" to the test block that's failing

kunicmarko20 commented 5 years ago

Green :tada: now to address suggestions by @angelsk

kunicmarko20 commented 5 years ago

Should be good now, thank you @jmsche and @angelsk!

kunicmarko20 commented 5 years ago

https://github.com/FriendsOfSymfony/FOSCKEditorBundle/releases/tag/2.0.1