There are still @todos in core referring to #2109287: Replace list_themes() with a service.. That went in, they never got done.

CommentFileSizeAuthor
#1 list-themes-2188991-1.patch9.56 KBtim.plunkett
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tim.plunkett’s picture

Status: Active » Needs review
FileSize
9.56 KB

There are other places in OO code where list_themes() is used, that's an easy way to spot no unit test coverage.

amateescu’s picture

Status: Needs review » Reviewed & tested by the community

Nice find! I hope the bot will agree with an early RTBC :)

sun’s picture

Looks good.

That said, this patch introduces a new usage of $theme->filename which is an inane property that should (at least) be $uri, as I also discovered in #2188661: Extension System, Part II: ExtensionDiscovery, but I hope/assume that we'll be able to get rid of all of these bogus $filename instances in one go later on. (And for that, ->filename is most likely easier to grep than ->uri even ;))

Anyway, no reason to hold up this nice cleanup patch for that. :)

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

Status: Fixed » Closed (fixed)

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