Per #2474121-9: CacheableMetadata should get BubbleableMetadata's merge/applyTo/createFromRenderArray/createFromObject methods/#2474121-10: CacheableMetadata should get BubbleableMetadata's merge/applyTo/createFromRenderArray/createFromObject methods/#2335661-69: Outbound path & route processors must specify cacheability metadata.

CommentFileSizeAuthor
#6 interdiff.txt8.15 KBwim leers
#6 2475397-6.patch8.66 KBwim leers
#3 2475397-3.patch1.03 KBwim leers
#1 outbound_processors_cacheability-2335661-71.patch46.33 KBwim leers

Comments

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Active » Needs review
StatusFileSize
new outbound_processors_cacheability-2335661-71.patch46.33 KB
fabianx’s picture

fabianx commented
Status: Needs review » Needs work

I don't think this is the right patch file ...

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs work » Needs review
StatusFileSize
new 2475397-3.patch1.03 KB

D'oh.

fabianx’s picture

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

RTBC, looks great!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs tests

Shouldn't we be testing this?

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: Tiny follow-up for #2474121 » Tiny follow-up for #2474121 + unit test coverage for (Bubbleable|Cacheable)Metadata::merge()
Status: Needs work » Needs review
Issue tags: -Needs tests +blocker
StatusFileSize
new 2475397-6.patch8.66 KB
new interdiff.txt8.15 KB

Yes. We weren't because there was a @todo to test the ::merge() function, and we're still bringing that along. Let's just unit test as much as we can.

fabianx’s picture

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

Unless Alex wants a test-only patch this should be good to go.

Thanks, Wim!

alexpott’s picture

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

Looks great. Thanks. Committed a798443 and pushed to 8.0.x. Thanks!

  • alexpott committed 77f2abc on 8.0.x 
    Issue #2475397 by Wim Leers: Tiny follow-up for #2474121 + unit test...

Status: Fixed » Closed (fixed)

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