1up-lab / OneupFlysystemBundle

A Flysystem integration for your Symfony projects.
MIT License
634 stars 118 forks source link

allow symfony 5 #200

Closed mihaileu closed 4 years ago

mihaileu commented 4 years ago

fixes #199

tacman commented 4 years ago

I think these dependencies also need to be updated to inclde ^5.0 in require-dev:

    "symfony/templating": "^3.3|^4.0",
    "symfony/translation": "^3.3|^4.0",
garak commented 4 years ago

It looks like you need to add a requirement for symfony/yaml

kuraobi commented 4 years ago

It looks like you need to add a requirement for symfony/yaml

Yep, PHPStan used to require symfony/yaml but not anymore so you need to add it manually. But you could also just merge #202 if you don't want to do the work twice, I have already fixed all the issues there.

bytehead commented 4 years ago

Thanks to both of you guys @mihaileu and @kuraobi! 🎉

tacman commented 4 years ago

I think Symfony 5 requires PHP 7.2.5+

https://symfony.com/doc/current/setup.html#technical-requirements

On Fri, Dec 6, 2019 at 10:40 AM David Greminger notifications@github.com wrote:

@bytehead requested changes on this pull request.

In .travis.yml https://github.com/1up-lab/OneupFlysystemBundle/pull/200#discussion_r354893402 :

@@ -19,17 +19,17 @@ matrix: allow_failures:

  • env: SYMFONY_VERSION=dev-master exclude:
    • php: 7.0
  • env: SYMFONY_VERSION=^4.0
    • php: 7.0
    • php: 7.1
  • env: SYMFONY_VERSION=^5.0

Code style - can you remove the spaces until the end of the line after 5.0 ?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/1up-lab/OneupFlysystemBundle/pull/200?email_source=notifications&email_token=AAEXIQNNIP4QX7STDSW4YPDQXJXAFA5CNFSM4JQOWRO2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCOIS7EI#pullrequestreview-328282001, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEXIQLBPO3PMG4FBUF537DQXJXAFANCNFSM4JQOWROQ .

bytehead commented 4 years ago

This should be possible, the requirement of this bundle is 7.1 at minimum (for Sf3/4).

tacman commented 4 years ago

Oh, I thought the travis file would need to match that version of PHP. Alas, I still don't understand travis, have yet to get my own bundle passing on travis. Sigh.

On Fri, Dec 6, 2019 at 10:48 AM David Greminger notifications@github.com wrote:

This should be possible, the requirement of this bundle is 7.1 at minimum (for Sf3/4).

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/1up-lab/OneupFlysystemBundle/pull/200?email_source=notifications&email_token=AAEXIQKN3DBAURDY3WQRXWTQXJX3LA5CNFSM4JQOWRO2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGEP2FA#issuecomment-562625812, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEXIQKY5N5SCR7BUZFL62DQXJX3LANCNFSM4JQOWROQ .

bytehead commented 4 years ago

I think with php: 7.2 you will get the newest version. Otherwise our tests would fail, but they actually pass also with Symfony 5 on PHP 7.2 😎

bytehead commented 4 years ago

3.3.0 is released 🎉