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

Drop Twig 1 support #238

Merged
merged 2 commits into from
Dec 12, 2019
Merged

Drop Twig 1 support #238

merged 2 commits into from
Dec 12, 2019

Conversation

FabienPapet
Copy link
Contributor

No description provided.

@FabienPapet FabienPapet changed the title Enable twig 3.0 support WIP Enable twig 3.0 support Nov 24, 2019
@FabienPapet FabienPapet changed the title WIP Enable twig 3.0 support Enable twig 3.0 support Dec 10, 2019
@FabienPapet
Copy link
Contributor Author

@florianeckerstorfer I'm not happy with that solution but I think it's the best way to have all twig versions working with any version of slugify.

This code works on twig 3. I didn't wrote any test, I don't know how to test it.

@florianeckerstorfer
Copy link
Member

Should we just drop support for Twig 2?

@FabienPapet
Copy link
Contributor Author

The real problem is twig 1 here I think. If I remember correctly , twig 2 is supporting old and new namespaces. We can make a version that supports only twig ~2.0|~3.0

What do you think ?

@florianeckerstorfer
Copy link
Member

@FabienPapet From my perspective this would be totally fine. Once we have Twig 3 support in, I would release a new major version. People who need support for old frameworks would need to use an old version of the library

@@ -22,9 +22,6 @@
"php": ">=7.0",
"ext-mbstring": "*"
},
"conflict": {
"twig/twig": ">=1,<1.38.1 || >=2,<2.12.1"
Copy link
Contributor

Choose a reason for hiding this comment

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

Since twig/twig is not in the require section, shouldn't this be something like this (in conflict)?

"twig/twig": "<2.12.1"

And I just realised I should've done the same for Symfony < 3.4, I'll make a PR

Choose a reason for hiding this comment

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

Yes, thanks

@florianeckerstorfer
Copy link
Member

Thanks for the PR, can you resolve the conflict, then I merge it and release a new version over the weekend

@FabienPapet FabienPapet changed the title Enable twig 3.0 support Drop Twig 1 support Dec 12, 2019
@florianeckerstorfer florianeckerstorfer merged commit 8232ea6 into cocur:master Dec 12, 2019
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.

3 participants