Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Twig deprecation notice #196

Merged
merged 4 commits into from
May 21, 2019
Merged

Twig deprecation notice #196

merged 4 commits into from
May 21, 2019

Conversation

emulienfou
Copy link
Contributor

This Pull Request fix all Twig deprecation notices when using Twig >= 2.7 who now use namespaces.
However to achieve this, the support for SF2.7, PHP5.x and Twig < 2.7 has been dropped.

@lsmith77
Copy link
Contributor

lsmith77 commented Apr 3, 2019

thank you for this!

I am a bit unsure about jumping to Twig 2.7 so quickly .. let me quickly collect some feedback via twitter

@lsmith77 lsmith77 mentioned this pull request Apr 3, 2019
@lsmith77
Copy link
Contributor

lsmith77 commented Apr 3, 2019

https://twitter.com/lsmith/status/1113341417856876545

@alcaeus
Copy link

alcaeus commented Apr 4, 2019

I am a bit unsure about jumping to Twig 2.7 so quickly .. let me quickly collect some feedback via twitter

For DoctrineBundle, we changed the constraint to also allow 1.x versions with namespaced classes: https://github.com/doctrine/DoctrineBundle/blob/90a7508c0da6b2c8add42f3f91020480f5d1ba2c/composer.json#L43 (twig is optional for DoctrineBundle, hence the combination of require-dev and conflict. In your case, this can easily be done in require.)

@lsmith77
Copy link
Contributor

lsmith77 commented Apr 4, 2019

ok .. didn’t realize that Twig 1.34+ has namespaced aliases .. @emulienfou could you look into supporting that version as well?

@emulienfou
Copy link
Contributor Author

Hi @lsmith77 working on it as soon as possible

@emulienfou
Copy link
Contributor Author

@lsmith77 I previously drop support of PHP5.6 and SF2.7 too.
Is this OK for you?

@lsmith77
Copy link
Contributor

lsmith77 commented Apr 4, 2019 via email

@tifabien
Copy link
Contributor

@lsmith77 Do you think you can merge this PR and make a release? Thanks a lot @emulienfou !

@lsmith77
Copy link
Contributor

thank you!

waiting for some feedback before making a release https://twitter.com/lsmith/status/1130747016190017536

@@ -13,13 +13,11 @@ cache:
- $HOME/.composer/cache/files

env:
- SYMFONY_VERSION=2.7.*
- SYMFONY_VERSION=3.0.*
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3.0.* does not make sense, as that's unmaintained. The default should be to not force a Symfony version. And then having some jobs forcing LTS versions

env: TWIG_VERSION=2.x
- php: 5.6
env: COMPOSER_FLAGS="--prefer-lowest"
env: TWIG_VERSION=2.7.*
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why 2.7.* ? That will be unmaintained as soon as Twig 2.8 is released.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the intention is to test the lower bound, but I guess then it should be 2.4

@lsmith77
Copy link
Contributor

@emulienfou are you motivated to work on the suggested improvements from @stof to the build matrix ?

@lsmith77
Copy link
Contributor

ok .. trying to implement @stof's feedback here #200

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants