Skip to content

Deprecate Environment::mergeGlobals()#4283

Merged
fabpot merged 1 commit intotwigphp:3.xfrom
fabpot:mergeGlobals
Sep 7, 2024
Merged

Deprecate Environment::mergeGlobals()#4283
fabpot merged 1 commit intotwigphp:3.xfrom
fabpot:mergeGlobals

Conversation

@fabpot
Copy link
Copy Markdown
Contributor

@fabpot fabpot commented Sep 7, 2024

No description provided.

@fabpot fabpot merged commit 7cd4352 into twigphp:3.x Sep 7, 2024
@fabpot fabpot deleted the mergeGlobals branch September 7, 2024 14:44
derrabus added a commit to symfony/symfony that referenced this pull request Sep 9, 2024
…abus)

This PR was merged into the 5.4 branch.

Discussion
----------

[TwigBridge] Avoid calling deprecated mergeGlobals()

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | Follows twigphp/Twig#4283
| License       | MIT

`getGlobals()` is documented as the official replacement for `mergeGlobals()` despite being flagged ``@internal``. I've left a comment on the upstream PR.

Commits
-------

2883331 [TwigBridge] Avoid calling deprecated mergeGlobals()
@ruudk
Copy link
Copy Markdown
Contributor

ruudk commented Sep 10, 2024

@fabpot It is a bit unfortunate, that this was merged while Symfony TwigBridge has not yet been tagged.

Given this is included in the security release 3.14, we cannot upgrade because of deprecated usage in TwigBridge:

Remaining indirect deprecation notices (109)
109x: Since twig/twig 3.14: The "Twig\Environment::mergeGlobals" method is deprecated.

Would it be an idea to get this tagged? symfony/symfony#58207

I know one solution could be to turn off detecting of indirect deprecations, but it's super valuable to detect deprecations in vendor and help to get those fixed.

@derrabus
Copy link
Copy Markdown
Contributor

@ruudk There's no shame in temporarily installing a dev snapshot of the Twig Bridge if that solves a problem for you.

@ruudk
Copy link
Copy Markdown
Contributor

ruudk commented Sep 10, 2024

True. But I'm raising it, as there might be more people getting this "problem".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

6 participants