Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

[TwigBundle] Use the apply tag instead of the filter tag#31290

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
fabpot merged 1 commit intosymfony:3.4fromgreg0ire:use-apply-instead-of-filter
Apr 28, 2019

Conversation

@greg0ire
Copy link
Contributor

@greg0iregreg0ire commentedApr 27, 2019•
edited
Loading

The filter has been deprecated in favor of the apply tag since Twig 2.9,
seehttps://twig.symfony.com/doc/2.x/tags/filter.html (apply does not
seem to have its own documentation page yet).

QA
Branch?3.4
Bug fix?yes
New feature?no
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed ticketsn/a
LicenseMIT
Doc PRn/a

⚠ Note: don't merge until Twig 2.9 and 1.40 are released, or this will block symfony releases ⚠

@greg0ire
Copy link
ContributorAuthor

The bug can be observed like this:

./phpunit src/Symfony/Bundle/TwigBundle/#!/usr/bin/env phpPHPUnit 6.5.14 by Sebastian Bergmann and contributors.Testing src/Symfony/Bundle/TwigBundle/.......................................................           55 / 55 (100%)Time: 733 ms, Memory: 26.00MBOK (55 tests, 190 assertions)Remaining direct deprecation notices (2)  2x: The "filter" tag is deprecated since Twig 2.9, use the "apply" tag instead.    1x in CacheWarmingTest::testCacheIsProperlyWarmedWhenTemplatingIsAvailable from Symfony\Bundle\TwigBundle\Tests\Functional    1x in CacheWarmingTest::testCacheIsProperlyWarmedWhenTemplatingIsDisabled from Symfony\Bundle\TwigBundle\Tests\Functional

@chalasr
Copy link
Member

chalasr commentedApr 27, 2019•
edited
Loading

This deprecation is breaking our test suite on 3.4https://travis-ci.org/symfony/symfony/jobs/525267589#L5408 (not master because the test is marked a legacy there). Bumping the twig requirement to2.9 is not possible on lower branches (even for master, twig 2.9 is not released yet).
We need to use thefilter tag when theapply one is not available

@greg0ire
Copy link
ContributorAuthor

I contributed it on that branch because I did not observe the deprecation on 3.4, but I must have forgotten to update my vendors. The fix will be harder on 3.4 indeed: is there a way to detect if a tag is available from Twig?

@chalasr
Copy link
Member

There is no AFAIK :/
I thought about registering an internal twig function on 3.4 which would check the existence of a twig 2.9 class e.g.ApplyTokenParser, that could work. But@xabbuh told me that he will be able to look into this bug tonight so I suggest to wait here :)

greg0ire reacted with thumbs up emoji

@greg0ire
Copy link
ContributorAuthor

In case he does find a solution, what should we do with my PR? Close it and let his solutionapply 🥁 ? Or is it best to bump the dependency?

@chalasr
Copy link
Member

I think we would better not bump it (in case Twig 2.9 is not ready in time for the Symfony 4.3 releases) but I don't know tbh :) Let's keep this open until we have more inputs

greg0ire reacted with thumbs up emoji

@fabpot
Copy link
Member

You can bump the minimum version. I will release the new versions of Twig 1.x and 2.x in the next few days.

@fabpot
Copy link
Member

On 3.4 :)

greg0ire reacted with thumbs up emoji

@greg0iregreg0ireforce-pushed theuse-apply-instead-of-filter branch fromcc68892 to56fe300CompareApril 27, 2019 17:58
@greg0iregreg0ire changed the base branch frommaster to3.4April 27, 2019 17:58
The filter has been deprecated in favor of the apply tag since Twig 2.9,seehttps://twig.symfony.com/doc/2.x/tags/filter.html (apply does notseem to have its own documentation page yet).
@greg0iregreg0ireforce-pushed theuse-apply-instead-of-filter branch from56fe300 to9c11b98CompareApril 27, 2019 18:55
@greg0ire
Copy link
ContributorAuthor

cc@xabbuh

@greg0iregreg0ire changed the titleUse the apply tag instead of the filter tag[TwigBundle] Use the apply tag instead of the filter tagApr 27, 2019
@greg0ire
Copy link
ContributorAuthor

I added a warning above, cause I think this should not be merged yet, right?

⚠ Note: don't merge until Twig 2.9 and 1.40 are released, or this will block symfony releases ⚠

@chalasrchalasr added this to the3.4 milestoneApr 27, 2019
@fabpot
Copy link
Member

Thank you@greg0ire.

@fabpotfabpot merged commit9c11b98 intosymfony:3.4Apr 28, 2019
fabpot added a commit that referenced this pull requestApr 28, 2019
…greg0ire)This PR was merged into the 3.4 branch.Discussion----------[TwigBundle] Use the apply tag instead of the filter tagThe filter has been deprecated in favor of the apply tag since Twig 2.9,seehttps://twig.symfony.com/doc/2.x/tags/filter.html (apply does notseem to have its own documentation page yet).| Q             | A| ------------- | ---| Branch?       | 3.4| Bug fix?      | yes| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets | n/a| License       | MIT| Doc PR        | n/a⚠ Note: don't merge until Twig 2.9 and 1.40 are released, or this will block symfony releases ⚠Commits-------9c11b98 Use the apply tag instead of the filter tag
This was referencedMay 1, 2019
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@fabpotfabpotfabpot approved these changes

@chalasrchalasrchalasr approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

3.4

Development

Successfully merging this pull request may close these issues.

4 participants

@greg0ire@chalasr@fabpot@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp