Problem/Motivation

Follow-up from #2575105: Use cache collector for state, these are only requested on container build, so don't need to go into state which is more for frequently accessed things.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#5 3437162-nr-bot.txt90 bytesneeds-review-queue-bot

Issue fork drupal-3437162

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

catch created an issue. See original summary.

plopesc made their first commit to this issue’s fork.

plopesc opened merge request !7299

plopesc’s picture

plopesc
he/him
Primary language Spanish
commented
Status: Active » Needs review

Twig debug and related development toggles from "Development settings" page moved to raw key value storage as part of the MR.

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 3437162-nr-bot.txt90 bytes

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

plopesc’s picture

plopesc
he/him
Primary language Spanish
commented
Status: Needs work » Needs review

Conflicts resolved and tests are green again. Back to NR.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs Review Queue Initiative

So tested this one out by applying the MR locally and still using the new development setting for debugging

From what I can tell everything remained the same.

Refactoring appears fine to me.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Reviewed & tested by the community » Needs work

Thanks for working on this.

The update function should also delete those keys then from state.

Not sure if we should do a change record just in case someone wrote some custom code related to these keys or used drush sset in their workflow.

plopesc’s picture

plopesc
he/him
Primary language Spanish
commented
Status: Needs work » Needs review

Thank you for your feedback.

Obsolete state keys are removed as part of the post_update hook.
Created change record draft: https://www.drupal.org/node/3438802

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Deletion from state has been added

Reviewed the CR and it reads well. Always get a huge thank you for adding before/after examples

  • catch committed d31ade0f on 10.3.x 
    Issue #3437162 by plopesc, smustgrave, Berdir: Move twig_debug and other...

  • catch committed be5ddf73 on 11.x 
    Issue #3437162 by plopesc, smustgrave, Berdir: Move twig_debug and other...

  • catch committed 9283d590 on 10.3.x 
    Issue #3437162 by plopesc, smustgrave, Berdir: Move twig_debug and other...
catch’s picture

catch
he/him
Primary language English
commented
Version: 11.x-dev » 10.3.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 11.x and cherry-picked to 10.3.x, thanks! I have to resolve a couple of merge conflicts on the 10.3.x cherry-pick. Straightforward enough but let's hope they actually were (had to add an extra commit for one thing already).

catch closed merge request !7299

Status: Fixed » Closed (fixed)

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

moshe weitzman’s picture

moshe weitzman commented

I wish Drupal would make an API for stuff like this. Then callers like drush twig:debug would be isolated from implementation changes. Raw key-value is not enough of an API.