8000 Fix: Use correct deprecation_info for Twig callables by mrfsrf · Pull Request #3064 · timber/timber · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix: Use correct deprecation_info for Twig callables #3064

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

Merged
merged 3 commits into from
Dec 17, 2024

Conversation

mrfsrf
Copy link
Contributor
@mrfsrf mrfsrf commented Nov 20, 2024

Issue

Twig v.3.15. introduced some nice features. Testing it with Timber v.2.x. works fine. However, there is this annoying deprecation warning:

'twig/twig', '3.15', 'Using the "deprecated", "deprecating_package", and "alternative" options is deprecated, pass a "deprecation_info" one instead.')

$options['deprecated'] is deprecated since Twig 3.15.
From Changelog:
* Improve the way one can deprecate a Twig callable (use deprecation_info instead of the other callable options)

https://github.com/twigphp/Twig/blob/709fc6abcef276d40581be663c120307ccf0f2ce/src/AbstractTwigCallable.php#L46

Solution

updates Timber to use the new deprecation_info option introduced in Twig 3.15 for handling deprecate 8000 d features.
Question:
Why was the deprecated option used to signal Twig in the first place when Timber already provides the Helper::deprecated() function to handle deprecation warnings? This seems redundant, as Helper::deprecated() outputs its own warning messages. Or am I missing something?

Impact

Usage Changes

Considerations

Testing

@Levdbas
Copy link
Member
Levdbas commented Nov 23, 2024

Hi @mrfsrf ,

Thank you for this, since we are currently testing for as low as Twig 3.5.0 the tests are failing. @timber/rangers , should we bump Twig to this latest version for an upcoming release or do we want fallbacks in this PR to also support lower versions? Bumping would be the way to go in my opinion.

@gchtr
Copy link
Member
gchtr commented Dec 3, 2024

I agree to bump the required Twig version. I don’t see a reason why we should keep backwards compatibility for a change like this.

@nlemoine
Copy link
Member
nlemoine commented Dec 3, 2024

Bumping would be the way to go in my opinion.

Same here!

@Levdbas
Copy link
Member
Levdbas commented Dec 3, 2024

@gchtr , @nlemoine , added a commit that bumped the minimal version of Twig.

@coveralls
Copy link

Coverage Status

coverage: 88.182% (+0.2%) from 88.018%
when pulling 1e91ca5 on mrfsrf:fix/deprecation-info-for-twig-callables
into 0e65e54 on timber:2.x.

@Levdbas Levdbas merged commit 72a013e into timber:2.x Dec 17, 2024
15 of 26 checks passed
This was referenced Dec 17, 2024
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.

Deprecation of use of the "deprecated", "deprecating_package", and "alternative" options
5 participants
0