We shouldn't typehint classes like this. Makes testing harder.

Patch changes it to typehint the interface.

CommentFileSizeAuthor
#2 dont-typehint-classes-2.patch1.66 KBmsonnabaum
dont-typehint-classes.patch1.69 KBmsonnabaum
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Status: Needs review » Needs work

The last submitted patch, dont-typehint-classes.patch, failed testing.

msonnabaum’s picture

msonnabaum CreditAttribution: msonnabaum commented
Status: Needs work » Needs review
FileSize
dont-typehint-classes-2.patch1.66 KB
katbailey’s picture

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

Yar.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

This looks consistent with what I've seen in entity API, where we pass an interface around rather than a class to allow mocking or whatever, as long as it conforms to the interface. So makes sense to me.

Committed and pushed to 8.x. Thanks!

Status: Fixed » Closed (fixed)

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