search

sonata-project / SonataCoreBundle

[deprecated] SonataCoreBundle
MIT License
318 stars 138 forks source link

Fix phpunit setUp signature #729

Closed mazsudo closed 4 years ago

mazsudo commented 4 years ago

Subject

I am targeting this branch, because this does not include BC breack.

Changelog

### Changed
Switch to phpunit 8 setUp signature in tests
greg0ire commented 4 years ago

The changes look OK but you didn't actually switch to phpunit 8 in travis.yml

mazsudo commented 4 years ago

🤔 maybe i should drop support for php7.1? not sure it should be done in this PR? WDYT?

greg0ire commented 4 years ago

If it's a blocker for you, sure, go ahead! 7.1 is EOL.

mazsudo commented 4 years ago

seems that there is an issue on prefer-lowest case. composer does not respond...did you ever experience this @greg0ire ?

greg0ire commented 4 years ago

Yes, and I ended up bumping some dependencies IIRC, that can help. You could bump all Symfony deps to 4.3 (and drop 2.8 and 3.4), that should help and that's what we have started to do on the admin bundle: https://github.com/sonata-project/SonataAdminBundle/pull/5843

greg0ire commented 4 years ago

The new LTS we support is 4.3, so you will have to change 3.4 to 4.3 in .travis.yml, but let's see if the prefer-lowest job succeeds first.

greg0ire commented 4 years ago

It succeeds, you're almost there :muscle:

mazsudo commented 4 years ago

think we are not bad ;) except coveralls

greg0ire commented 4 years ago

That's because there is code that could be removed now, for instance https://github.com/sonata-project/SonataCoreBundle/blob/ec5f858ae434a8411894e11e668d714ce6593f71/src/CoreBundle/DependencyInjection/Compiler/FlashMessageCompilerPass.php#L22

mazsudo commented 4 years ago

let me have a look. I like bonus point and it's always good to practice and learn ;) quick question not related: guess for all bundles, Symfony deps will be bumped to 4.3 and remove php7.1 support? I can help for some of them. Is there any task list or something closed to?

greg0ire commented 4 years ago

Is there any task list or something closed to?

No but if you want, you can create one in a issue on https://github.com/sonata-project/dev-kit, that's what we usually do.

OskarStark commented 4 years ago

Good job so far 👍🏻 Thank you

mazsudo commented 4 years ago

according to https://github.com/sonata-project/SonataPageBundle/pull/1105, should I remove XliffValidatorTestCase here? Is this PR still relevant even if CoreBundle is dprecated?

greg0ire commented 4 years ago

Removing it would be a BC-break so no… as for this PR, I think we should merge as is if you and @core23 agree. There is probably no need to do the extra effort to remove the dead code.

greg0ire commented 4 years ago

Thanks @bmaziere