API page: https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Extension...

Enter a descriptive title (above) relating to function hook_modules_installed, then describe the problem you have found:

Using 8.0.0-beta12

Please indicate in the documentation which file this hook should be implemented.

Following the convention of hook_install, I implemented hook_modules_installed in the .install file where this hook did not work.

After implementing hook_modules_installed in the .module file, it worked as expected.

Please clarify this so future developers don't make this same mistake.

Thanks.

CommentFileSizeAuthor
#16 hook_modules_installed-2538158-15-D7.patch504 bytesjoyceg
#11 hook_modules_installed-2538158-11-D7.patch526 bytesjoyceg
#8 hook_modules_installed-2538158-8.patch599 bytesjoyceg
#5 hook_modules_installed-2538158-5.patch616 bytesjoyceg
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dreambubbler’s picture

dreambubbler CreditAttribution: dreambubbler as a volunteer and at Acquia commented
jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Issue tags: -Documentation, -Needs documentation +Novice

Good idea. I would have been caught by this too. Sounds like a good Novice issue -- should be pretty easy to patch this.

Note: looking at the code that invokes this, I can confirm it is being invoked with a standard moduleHandler->invokeAll() call, so yes it must be in the .module file to be invoked. And definitely we should document this.

Thanks!

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Issue tags: +Needs backport to D7

Probably also need to backport to D7 after we fix it in D8.

joyceg’s picture

joyceg CreditAttribution: joyceg commented
Assigned: Unassigned » joyceg

I am working on this issue.

joyceg’s picture

joyceg CreditAttribution: joyceg commented
FileSize
hook_modules_installed-2538158-5.patch616 bytes
joyceg’s picture

joyceg CreditAttribution: joyceg commented
Status: Active » Needs review
joyceg’s picture

joyceg CreditAttribution: joyceg commented
Status: Needs review » Active
joyceg’s picture

joyceg CreditAttribution: joyceg commented
FileSize
hook_modules_installed-2538158-8.patch599 bytes
joyceg’s picture

joyceg CreditAttribution: joyceg commented
Status: Active » Needs review
joyceg’s picture

joyceg CreditAttribution: joyceg commented
Status: Needs review » Active
joyceg’s picture

joyceg CreditAttribution: joyceg commented
FileSize
hook_modules_installed-2538158-11-D7.patch526 bytes

Adding the patch for Drupal 7.

joyceg’s picture

joyceg CreditAttribution: joyceg commented
Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, 11: hook_modules_installed-2538158-11-D7.patch, failed testing.

Status: Needs work » Needs review

joyceg queued 11: hook_modules_installed-2538158-11-D7.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 11: hook_modules_installed-2538158-11-D7.patch, failed testing.

joyceg’s picture

joyceg CreditAttribution: joyceg commented
Status: Needs work » Needs review
FileSize
hook_modules_installed-2538158-15-D7.patch504 bytes

Iam adding the new patch for Drupal 7.

Status: Needs review » Needs work

The last submitted patch, 16: hook_modules_installed-2538158-15-D7.patch, failed testing.

roderik’s picture

roderik
Primary language Dutch
Location Amsterdam,NL / Budapest,HU
CreditAttribution: roderik as a volunteer commented
Status: Needs work » Reviewed & tested by the community

...and reviewed.

I thought I read that that the test bot ignored patches ending in -D7, apparently that's old information.

So - #8 for D8
#16 or D7 included too.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented

In general, we work on issues one version at a time. So we would want to work on the Drupal 8 patch until Drupal 8 has been finalized, and then backport to Drupal 7. See
https://www.drupal.org/node/1319154#multiple-versions

Anyway, I agree that the patch in #8 is fine for Drupal 8, and #16 for Drupal 7.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Version: 8.0.x-dev » 7.x-dev
Status: Reviewed & tested by the community » Needs review
Issue tags: -Needs backport to D7

Docs changes are not frozen in beta. Committed b51dc5c and pushed to 8.0.x. Thanks!

  • alexpott committed b51dc5c on 8.0.x 
    Issue #2538158 by joyceg: Clarification to where hook_modules_installed...
cilefen’s picture

cilefen CreditAttribution: cilefen commented
Title: Clarification to where hook_modules_installed should be implemented » Clarify in which file hook_modules_installed should be implemented
Version: 7.x-dev » 8.0.x-dev

The hook exists in Drupal 8 so it must be patched there first.

cilefen’s picture

cilefen CreditAttribution: cilefen commented
Version: 8.0.x-dev » 7.x-dev

Oops.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Status: Needs review » Reviewed & tested by the community

#16 patch is RTBC for Drupal 7.

David_Rothstein’s picture

David_Rothstein CreditAttribution: David_Rothstein as a volunteer commented
Status: Reviewed & tested by the community » Fixed

Committed to 7.x - thanks!

  • David_Rothstein committed d1bef53 on 7.x 
    Issue #2538158 by joyceg: Clarify in which file hook_modules_installed...

Status: Fixed » Closed (fixed)

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