In reading the documentation for hook_install_tasks, it seems the preferred location for hook_install* items is the .install file. It seems that all of commons hook_install* ship in the .profile.

A comment also suggests an improvement, inline with typical drupal module best practices.

However, depending on your circumstances, I think you generally want to put hook_install_tasks in your .install profile because the .profile file is run ON EVERY PAGE LOAD. (see book 'Drupal 7 Module Development', pg 359 and http://drupal.org/node/509398 "always loading the install profile code").

At least worth the investigation/refactoring.

CommentFileSizeAuthor
#2 move-install-functions-to-install-file-2059695-2.patch50.87 KBdevin carlson

Comments

ezra-g’s picture

ezra-g commented

Worth considering, though I'd expect that with opcode caching enabled, which is a best practice, that this would have little to no measurable impact on performance.

ezra-g’s picture

ezra-g commented
Issue summary: View changes

edits

devin carlson’s picture

devin carlson commented
Issue summary: View changes
Status: Active » Needs review
StatusFileSize
new move-install-functions-to-install-file-2059695-2.patch50.87 KB

A patch to move the install_* functions from commons.profile to commons.install.

devin carlson’s picture

devin carlson commented
Status: Needs review » Fixed

Tested #2 by performing multiple fresh installs of Commons and verifying that all of the install functionality continued to function properly (enabling Acquia Connector, creating first group, adding example content, etc).

Committed #2 to Commons 7.x-3.x.

http://drupalcode.org/project/commons.git/commit/800e85b

Status: Fixed » Closed (fixed)

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