The twig.engine is manually included for BC reasons in Drupal\Core\Template\TwigEnvironment. See #2591515: Move twig_without() to the TwigExtension where all the other filters are and deprecate for more information.
Remove the manual inclusion of twig.engine as well as the deprecated twig_without() procedural function in twig.engine, which was the purpose of the include.
Passing tests will allow us to know for sure that the deprecated functions in twig.engine aren't being used.
None
Removes the already deprecated twig_without() proceedural function in twig.engine.
None
The deprecated twig_without() proceedural function in twig.engine has been removed.
| Comment | File | Size | Author |
|---|---|---|---|
| #19 | drupal-remove-manual-inclusion-twig-3011393-19.patch | 5.6 KB | spokje |
Comments
Comment #2
andypostComment #3
scott_euser commentedRemoved in patch so we can at least see what failures this results in (Drupal\KernelTests\Core\Theme\TwigEnvironmentTest passes fine).
Comment #4
scott_euser commentedComment #5
scott_euser commentedComment #6
andypostProper ststus when there's patch https://www.drupal.org/issue-queue/status
Comment #7
scott_euser commentedSorry about that! Updated patch which should solve the issue the test runner found.
Comment #8
scott_euser commentedComment #9
volegerThanks for the patch. But 9.x not branched yet. Leaving active status till active dev phase.
Comment #11
krzysztof domańskiNeeds a reroll.
Comment #12
TylerMarshall commentedI've re-rolled the patch against the new 9.0.x branch. Let me know if I need to make any improvements. Tests pass locally.
Comment #13
scott_euser commentedCan you add an interdiff please? It is difficult to review what you have changed otherwise.
Comment #14
markhalliwellRerolls, inherently, make interdiffs impossible to create due to the divergence of code between the patches, hence the need for a re-roll.
How is reviewing 11 added/removed lines manually difficult? I visually checked #12 against #7; it's fine.
---
Leaving as CNW since this should probably go ahead and also remove the
twig_without()procedural function intwig.engine, which is the only reasontwig.enginewas being included anyway.From the original IS (updated it): "know for sure that the deprecated functions in
twig.enginearen't being used"The fact that the tests pass in #12 is proof that it's not being used anywhere.
I don't see any follow-up issue for removing that function, so we might as well do that here too.
Comment #15
markhalliwellComment #16
scott_euser commentedMakes it easy to see when viewing on a mobile phone what's changed since my previous patch is all. Didn't realise it was going to cause such a fuss, or I wouldn't have asked, sorry.
Comment #17
spokjePatch with #14 addressed.
Comment #18
spokjeTestbot likes it => Needs Review
Created a draft CR here: https://www.drupal.org/node/3096454
If somebody could have a quick look at it, that would be grand since I've never written one before.
Comment #19
spokjeMeh, forgot to remove a now obsolete
use...Comment #20
spokjeComment #21
longwaveThis looks good to me and is some nice cleanup for Drupal 9.
Comment #23
catchCommitted eadc537 and pushed to 9.0.x. Thanks!