Per #3069696-42: Remove BC layers from the entity system, we need to remove the NULL value from $memory_cache in the ConfigEntityStorage constructor, and remove the |null from the @param declaration.

CommentFileSizeAuthor
#9 interdiff_5-9.txt3.53 KBneelam_wadhwani
#9 3118998-9.patch3.53 KBneelam_wadhwani
#5 interdiff_2-5.txt739 byteskatherined
#5 3118998-5.patch2.79 KBkatherined
#2 3118998-2.patch1.92 KBkatherined

Comments

katherined created an issue. See original summary.

katherined’s picture

katherined
she/her
Primary language English
commented
Status: Active » Needs review
StatusFileSize
new 3118998-2.patch1.92 KB
katherined’s picture

katherined
she/her
Primary language English
commented
Issue summary: View changes
phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Title: Remove NULL default from ConfigEntityStorage » Remove null default for $memory_cache in entity storage handler constructors
Status: Needs review » Needs work

Nice work! This looks great.

I did some poking around in the rest of the entity system and discovered that there are a few other places that were missed. Let's also fix this for:

  • \Drupal\Core\Entity\KeyValueStore\KeyValueEntityStorage
  • \Drupal\Core\Entity\Sql\SqlContentEntityStorage

Re-titling accordingly.

katherined’s picture

katherined
she/her
Primary language English
commented
StatusFileSize
new 3118998-5.patch2.79 KB
new interdiff_2-5.txt739 bytes

It looks like those were addressed, except for one of the comments.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Needs work » Needs review
effulgentsia’s picture

effulgentsia commented
Status: Needs review » Needs work

Let's also get the @param declaration of ContentEntityStorageBase fixed. Other than that, I think this looks good.

neelam_wadhwani’s picture

neelam_wadhwani commented
Assigned: Unassigned » neelam_wadhwani
neelam_wadhwani’s picture

neelam_wadhwani commented
StatusFileSize
new 3118998-9.patch3.53 KB
new interdiff_5-9.txt3.53 KB

Kindly review patch.

neelam_wadhwani’s picture

neelam_wadhwani commented
Assigned: neelam_wadhwani » Unassigned
Status: Needs work » Needs review
phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Needs review » Reviewed & tested by the community

Looks good to me.

alexpott’s picture

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

Committed ba351b4 and pushed to 9.0.x. Thanks!

  • alexpott committed ba351b4 on 9.0.x 
    Issue #3118998 by katherined, neelam_wadhwani, phenaproxima,...

Status: Fixed » Closed (fixed)

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