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
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Status: Active » Needs review
FileSize
outbound_processors_cacheability-2335661-71.patch46.33 KB
Fabianx’s picture

Fabianx CreditAttribution: Fabianx for Acquia commented
Status: Needs review » Needs work

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

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Status: Needs work » Needs review
FileSize
2475397-3.patch1.03 KB

D'oh.

Fabianx’s picture

Fabianx CreditAttribution: Fabianx for Acquia commented
Status: Needs review » Reviewed & tested by the community

RTBC, looks great!

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 tests

Shouldn't we be testing this?

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia 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
FileSize
2475397-6.patch8.66 KB
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 CreditAttribution: Fabianx for Acquia 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 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three 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.