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

Compatibility fixes to support symfony 3.0 #412

Merged
merged 1 commit into from
Dec 20, 2015

Conversation

lashus
Copy link
Contributor

@lashus lashus commented Dec 16, 2015

Please check it down. It's been working for my sf 3.0 and sf 2.6

@dkisselev
Copy link
Contributor

It looks like your rebase didn't pull the recent changes in this repository into yours (so all the commits that happened here are in this PR as requested new commits)

Try doing a `git pull --rebase' so that those changes get pulled in and your commit is applied on top of it.

@lashus lashus closed this Dec 16, 2015
@lashus lashus reopened this Dec 16, 2015
@lashus lashus force-pushed the develop branch 2 times, most recently from cc40021 to 39b66d3 Compare December 16, 2015 22:31
@lashus
Copy link
Contributor Author

lashus commented Dec 16, 2015

Ok I think I've done it :)

@dkisselev
Copy link
Contributor

Looks good!

The CI tests are failing for your changes though, could you look over the tests to either update them or fix the issues?

@lashus
Copy link
Contributor Author

lashus commented Dec 17, 2015

Ok, any idea why it fails on php 5.3.3? I guess it might be something with autoloaders?

@florianeckerstorfer
Copy link
Member

Symfony 3 requires PHP 5.5.9. Maybe we should put out a new major version to be compatible.

🐯

On 17.12.2015, at 11:41, lashus [email protected] wrote:

Ok, any idea why it fails on php 5.3.3? I guess it might be something with autoloaders?


Reply to this email directly or view it on GitHub.

@lashus
Copy link
Contributor Author

lashus commented Dec 17, 2015

@florianeckerstorfer we definitely should. Although with the configuration I provided it should work both with symfony 2.6+ (with legacyHelper) and 3.0. Symfony 3 forces composer to use php > 5.5.9 so it won't be a problem. I'm fixing test issues.

@dkisselev
Copy link
Contributor

Your new commits seem to have un-rebased your changes, so all the old commits are back :(

I think the current solution is pretty good - detects the Symfony version and uses the appropriate syntax so deprecations are avoided as well as maintaining two release branches.

Add support for symfony 3.0

fix tests

fix tests
@lashus
Copy link
Contributor Author

lashus commented Dec 17, 2015

Fixed :) Thanks.

@dkisselev
Copy link
Contributor

I just noticed that Scritinizer has identified a whole bunch of deprecation issues that I introduced in my commits,

Apparently Twig_SimpleFunction is already deprecated and equivalent to Twig_Function (the documentation on this is very unclear). I believe that might need to be fixed. @florianeckerstorfer I noticed that you're burning through all the old tickets right this second, but I'm going to take a look right now to see if replacing SImpleFunction with Function still works.

@florianeckerstorfer
Copy link
Member

Great thanks a lot for your hard work. Yeah I cleaned up a few of the old issues, hopefully I can test and merge this one tomorrow.

@dkisselev
Copy link
Contributor

Ok, so it looks like Scrutinizer is jumping the gun a bit on those notices.

Basically, Twig wasn't happy with the API that they had for Twig_Function, so they're pushing everyone onto SimpleFunction for 2.0, which follows the new API, and immediately deprecated it. In Twig 3.0, they're going to bring back Twig_Function with the new API.

Technically the deprecation notice isn't wrong, but the current implementation is the recommended way of doing things, and will remain that way until Twig 2.0 comes out when we can use Twig_Function again. We physically cannot fix the issue right now because Twig_Function doesn't exist in 1.x anymore.

florianeckerstorfer pushed a commit that referenced this pull request Dec 20, 2015
Fix compatibility with Symfony 3.0
@florianeckerstorfer florianeckerstorfer merged commit ad5ee4d into braincrafted:develop Dec 20, 2015
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