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

Comments

joelpittet’s picture

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

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

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet 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
CreditAttribution: star-szr as a volunteer 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
CreditAttribution: xjm at Acquia 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
Primary language English
Location Vancouver
CreditAttribution: joelpittet 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
Primary language English
Location Vancouver
CreditAttribution: joelpittet 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 CreditAttribution: sdstyles at FFW commented
Status: Needs work » Needs review
FileSize
2515018-8.patch19.52 KB
joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet 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 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three 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.