Closed (fixed)
Project:
Drupal core
Version:
8.0.x-dev
Component:
base system
Priority:
Major
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
15 Apr 2015 at 14:56 UTC
Updated:
30 Apr 2015 at 13:04 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
pwolanin commentedComment #2
wim leersEither
CacheableMetadataorCacheabilityMetadataare the names that come to mind.Comment #3
pwolanin commentedComment #4
pwolanin commentedwhoops - that had some other crap in it. Here's the right patch.
Comment #5
wim leersThis is not an interface, but a class :)
Why is this interface necessary? Why would anyone ever want to swap out the
CacheableMetadataclass?Comment #6
pwolanin commentedok.
Comment #7
wim leersBlocks #2335661: Outbound path & route processors must specify cacheability metadata.
Patch looks great; now just a matter of moving a bunch of
BubbleableMetadataTesttoCacheableMetadataTest.s/storing and fetching/passing/
But that's a nit.
Comment #10
pwolanin commentedCache::mergeContexts() calls out to a service, so can't unit test it.
Comment #12
wim leersFailing tests :(
Could use a data provider.
Comment #13
pwolanin commented@Wim Leers - no, I considered a data provider, and I don't think it makes sense because I'm accumulating tags - in other words, the correctness of each expected result depends on the prior ones. I don't think that's how data providers usually work.
Comment #15
wim leersOh, sorry, missed that.
Comment #16
alexpottThis issue addresses a major bug and is allowed per https://www.drupal.org/core/beta-changes. Committed a0ee087 and pushed to 8.0.x. Thanks!
Fixed a capitalisation thing on commit.
Comment #18
fabianx commented+1 to this change!