Problem/Motivation

Typehinting with interfaces is in general the correct thing to do.
See #2271005-40: Rename Language module's LanguageInterface to ConfigurableLanguageInterface and Language to ConfigurableLanguage point 3.

Proposed resolution

type hint hooks with interface: ConfigurableLanguageInterface instead of LanguageEntity/ConfigurableLanguage

Remaining tasks

Contributor tasks needed
Task Novice task? Contributor instructions Complete?
Create a patch Instructions

User interface changes

No.

API changes

Technically yes, but it should be low impact.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jmolivas’s picture

Status: Active » Needs review
FileSize
1.99 KB
35.85 KB

Working on this Issue at TCDrupal

Replace type-hints from ConfigurableLanguage to ConfigurableLanguageInterface

jmolivas’s picture

FileSize
1.99 KB

Regenerate patch since related issue was fixed.

Did not upload interdiff since file was not changed

The last submitted patch, 1: 2316561-1.patch, failed testing.

The last submitted patch, 1: 2316561-1.patch, failed testing.

YesCT’s picture

Issue tags: +TCDrupal 2014
YesCT’s picture

Status: Needs review » Reviewed & tested by the community

no unneeded changes, still applies, looks good.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: -API change

Let's fix block_configurable_language_delete, language_configurable_language_delete, and node_configurable_language_delete too.

This is not an API change since it is a relaxation of the typehinting.

jmolivas’s picture

Status: Needs work » Needs review
FileSize
3.38 KB
5.35 KB

Fixing as request by @alexpott:

core/modules/block/block.module => block_configurable_language_delete
core/modules/language/language.module => language_configurable_language_delete
core/modules/node/node.module => node_configurable_language_delete too

jmolivas’s picture

My bad should be 8 and not 5 on the file names

* interdiff-2-8.txt
* 2316561-8.patch

YesCT’s picture

Status: Needs review » Reviewed & tested by the community

language_configurable_language_insert() also, ok.

used
ag 'function .*\(.*ConfigurableLanguage ' core/*

to check if we got them all. looks good again.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 4d86f97 and pushed to 8.0.x. Thanks!

  • alexpott committed 4d86f97 on 8.0.x
    Issue #2316561 by jmolivas | YesCT: Type hint hooks with interface:...

Status: Fixed » Closed (fixed)

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