While reviewing the patch at #2871591: Allow ComplexData in TypedData to specify computed properties that should be exposed in normalization and other contexts, I got very confused by the code I was reading. Turns out a $denormalized variable should've been called $normalized.

CommentFileSizeAuthor
#2 2916025-2.patch1.32 KBwim leers

Comments

Wim Leers created an issue. See original summary.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Active » Needs review
Issue tags: +API-First Initiative
StatusFileSize
new 2916025-2.patch1.32 KB
amateescu’s picture

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

Makes perfect sense :)

  • xjm committed 58c4686 on 8.5.x 
    Issue #2916025 by Wim Leers: Rename $denormalized to $normalized in \...

  • xjm committed 0ab5923 on 8.4.x 
    Issue #2916025 by Wim Leers: Rename $denormalized to $normalized in \...
xjm’s picture

xjm
she/her
Primary language English
commented
Version: 8.5.x-dev » 8.4.x-dev
Status: Reviewed & tested by the community » Fixed

Ha, that is indeed confusing.

I checked for other instances of $denormalized in case this was a situation where the same bug got copied along the inheritance chain:
grep -r "\$denormalized\b" *
All the remaining results were in tests and appeared at a glance to refer to denormalized values, rather than normalized ones. So the scope seems to be complete.

Committed to 8.5.x. Since this is a local variable, backporting the fix wouldn't have any disruption except that child implementations that copied the same confusing name would now additionally have a variable name that was out of sync, but that's not an actual disruption, so I also cherry-picked this to 8.4.x. Thanks!

Status: Fixed » Closed (fixed)

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