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

[quick fix] don't use twig version higher then 1.16.2 #9

Closed
wants to merge 1 commit into from

Conversation

adam187
Copy link

@adam187 adam187 commented Nov 1, 2014

Liniter is no longer compatible with twig 1.16.2

Since arguments changed for protected method getTestNodeClass of Twig_Extension_Core
https://github.com/asm89/twig-lint/blob/master/src/Asm89/Twig/Lint/Extension/StubbedCore.php#L30
https://github.com/twigphp/Twig/blob/v1.16.2/lib/Twig/Extension/Core.php#L341

Also some tests starts to faling for twig 1.16.2

since it breaks compatibility
@asm89
Copy link
Owner

asm89 commented Nov 19, 2014

Hm, I think we might be able to fix this. If we can figure out the Twig version we can load a class with the appropriate method.

Nasty BC break. :(

@adam187
Copy link
Author

adam187 commented Nov 19, 2014

@asm89 Yes, I thought about that too, i event started to work on PR with this kind of fix but there was also some other errors in unit tests so i made this quick temporary fix.

@tolry
Copy link
Contributor

tolry commented Jan 18, 2015

I am also having trouble with current twig versions (see travis runs on https://github.com/simplethings/SimpSpector/pull/89).

@asm89 do you favor a solution where the library supports both, the old and new method declaration?

Or would it also be acceptable to restrict the twig version appropriately? Make two releases of twig-lint, one compatible with older twig versions and one compatible with the new ones?

I would be willing to work on this issue 😄

@tolry
Copy link
Contributor

tolry commented Jan 18, 2015

btw: this is the commit twigphp/Twig@0082e05 introducing the change

@tolry tolry mentioned this pull request Feb 9, 2015
@DavidBadura
Copy link

any news?

@asm89
Copy link
Owner

asm89 commented Jul 29, 2015

I merged the PR from @tolry.

@asm89 asm89 closed this Jul 29, 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.

None yet

4 participants