Problem/Motivation

\Drupal\Core\Cache\Cache::keyFromQuery is an old weird method for cache key generation from the forum migration that has been replaced by better methods. No uses have existed in core since 2015 and the only use in contrib never got a release so its a solid cleanup.

Steps to reproduce

Its just removing cruft.

Proposed resolution

Deprecate and remove.

Remaining tasks

User interface changes

n/a

API changes

removes dead cache class method so technically.

Data model changes

n/a

Release notes snippet

tbd

CommentFileSizeAuthor
#9 interdiff_6-9.txt1.58 KBspokje
#9 3308507-9.patch2.28 KBspokje
#6 3308507-6-10x.patch2.28 KBneclimdul
#3 3308507-3-10x.patch1.25 KBneclimdul
#2 3308507-1.patch2.39 KBneclimdul
#2 3308507-1-10.x.patch1.07 KBneclimdul

Comments

neclimdul created an issue. See original summary.

neclimdul’s picture

neclimdul
He/Him
commented
Status: Active » Needs review
StatusFileSize
new 3308507-1-10.x.patch1.07 KB
new 3308507-1.patch2.39 KB

boom, easy patch. Going bold since this is just some really old dead code and patches for a 9.5 removal?

neclimdul’s picture

neclimdul
He/Him
commented
StatusFileSize
new 3308507-3-10x.patch1.25 KB

right, clean up the use. good catch testbot.

spokje’s picture

spokje
Primary language Dutch
commented
Status: Needs review » Reviewed & tested by the community

- Rationale for deprecating/removing explained in IS.
- Code changes make sense.
- TestBot is green.
- Code changes and green TestBot show that the method isn't used in core.

The only thing I'm wondering about is if were still on time to do this deprecation/removal in 9.5/10.0 or did we miss the window and this has to be 10.0/10.1.

RTBC for me and we'll await the verdict of the core committers on my last remark,

catch’s picture

catch
he/him
Primary language English
commented
Version: 9.5.x-dev » 10.1.x-dev
Status: Reviewed & tested by the community » Needs work

Much as I'd love to be rid of it, this needs to be 10.1 for removal in 11.0.

neclimdul’s picture

neclimdul
He/Him
commented
Issue summary: View changes
Status: Needs work » Needs review
StatusFileSize
new 3308507-6-10x.patch2.28 KB

Figured but wanted to try. Here's the deprecation for 10.1. Removal will have to wait.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs review » Needs work
Issue tags: +Needs change record

The @see references should point to a change record, not an issue.

spokje’s picture

spokje
Primary language Dutch
commented
Issue tags: -Needs change record

Added draft CR

spokje’s picture

spokje
Primary language Dutch
commented
Status: Needs work » Needs review
StatusFileSize
new 3308507-9.patch2.28 KB
new interdiff_6-9.txt1.58 KB

Updated deprecation message to point to CR instead of this issue.

  • catch committed afc9dc2 on 10.1.x 
    Issue #3308507 by neclimdul, Spokje, longwave: Remove Cache::...
catch’s picture

catch
he/him
Primary language English
commented
Status: Needs review » Fixed

Argghh I got confused about the issue status while reviewing the patch again and committed this from needs review. However since it was already RTBC before and the only changes have been version numbers and where the link points to.... going to leave it in.

spokje’s picture

spokje
Primary language Dutch
commented

Works for me ;)

Status: Fixed » Closed (fixed)

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