Problem/Motivation

@timplunkett noticed most of core uses instanceof not instanceOf
#2506151-20: Make the Twig extension link() accept Attribute objects

$ ag instanceOf --ignore-dir vendor | wc -l
34
$ ag instanceof --ignore-dir vendor | wc -l
669

Proposed resolution

Be consistent:)

Remaining tasks

User interface changes

API changes

Data model changes

Follow-up to #2506151: Make the Twig extension link() accept Attribute objects

CommentFileSizeAuthor
#8 2515018-8.patch19.52 KBsdstyles
#2 2515018-2.patch18.75 KBjoshi.rohit100

Comments

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Issue summary: View changes
joshi.rohit100’s picture

joshi.rohit100
he/him
Primary language Hindi
Location Delhi, India
commented
StatusFileSize
new 2515018-2.patch18.75 KB
joelpittet’s picture

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

Thank you @joshi.rohit100. I'm not sure how disruptive this patch will be but looks like you got all of them!

star-szr’s picture

star-szr
he/him
Primary language English
commented
Issue tags: +Needs beta evaluation

It's a normal task so I think it needs one of these.

xjm’s picture

xjm
she/her
Primary language English
commented
Status: Reviewed & tested by the community » Postponed
Issue tags: +rc target
Related issues: +#1518116: [meta] Make Core pass Coder Review, +#1299710: [meta] Automate the coding-standards part of patch review

Thanks everyone!

This issue is purely coding standards improvements with no prioritized changes, so it should be postponed during the beta. However, since there is virtually no risk from this patch, I think this one is safe as an RC target. Postponing as such.

This issue also follows the preferred issue scoping for coding standards issues: it fixes all instances to conform to a certain rule, instead of trying to fix different standards within a certain component or file.

Ideally, we would also add a rule for this once #1299710: [meta] Automate the coding-standards part of patch review is possible.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Postponed » Needs review
Issue tags: -Needs beta evaluation

Un-Postponing because in RC.

Needs review to check if we missed any.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Needs review » Needs work

This patch failed in a few places and missing 2 new ones. With the patch leaves 7 unfixed.

sdstyles’s picture

sdstyles commented
Status: Needs work » Needs review
StatusFileSize
new 2515018-8.patch19.52 KB
joelpittet’s picture

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

Thank you @sdstyles, I checked and it looks like you got them all:)

alexpott’s picture

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

Committed 1cd13a8 and pushed to 8.0.x. Thanks!

  • alexpott committed 1cd13a8 on 8.0.x 
    Issue #2515018 by joshi.rohit100, sdstyles, joelpittet: Lowercase the...

Status: Fixed » Closed (fixed)

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