Problem/Motivation

  public function __construct($root, \Twig_LoaderInterface $loader = NULL, $options = array()) {
    // @todo Pass as arguments from the DIC.
    $this->cache_object = \Drupal::cache();

Proposed resolution

Inject it

Remaining tasks

User interface changes

API changes

Data model changes

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Task because this is a code improvement.
Issue priority Normal because is a code cleanup that improves the testability of the class.
Disruption Really unlikely disruption, as someone would have to override the TwigEnvironment class.
CommentFileSizeAuthor
#1 inject_the_cache_bin-2539258-1.patch1.86 KBcilefen
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

cilefen’s picture

cilefen CreditAttribution: cilefen commented
Status: Active » Needs review
FileSize
inject_the_cache_bin-2539258-1.patch1.86 KB
dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer commented
Status: Needs review » Reviewed & tested by the community

Perfect

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs beta evaluation

A normal task without a beta evaluation can't be committed. I know it feels like busy-work but if we don't apply the policy to all issues then it is perceived to be unfair.

cilefen’s picture

cilefen CreditAttribution: cilefen commented
Issue summary: View changes

I'm not sure about the contrib disruption.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer commented
Issue summary: View changes
Status: Needs work » Reviewed & tested by the community
Issue tags: -Needs beta evaluation

Updated the contrib disruption.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Reviewed & tested by the community » Fixed

I agree that extending TwigEnvironment atm would be close to 0 and having non injected code in core encourages others to do it when we really want injected services everywhere. Let's do this. Committed be3b533 and pushed to 8.0.x. Thanks!

  • alexpott committed be3b533 on 8.0.x 
    Issue #2539258 by cilefen: Inject the cache bin into TwigEnvironment

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.