Since Mollom doesn't necessarily invoke its hooks on every single page request, it would be nice if integrating modules (D7 or later) could put their mollom hooks into a modulename.mollom.inc due to support from hook_hook_info(). This also helps separate code logically, as well as give a quick indication if a module supports Mollom or not.
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | mollom.hook-info.7.patch | 1.65 KB | sun |
| #6 | mollom.hook-info.6.patch | 944 bytes | sun |
| #2 | mollom.hook-info.2.patch | 4.3 KB | sun |
| #1 | 1165488-mollom-hook-info.patch | 5.08 KB | dave reid |
Comments
Comment #1
dave reidPatch for review. No need for tests since we natively test this with mollom_test.mollom.inc.
Comment #2
sunThanks, did a couple of tweaks.
Comment #3
sunI'm postponing this on #446994: Implement a generic Mollom PHP class
Comment #4
dave reidUm, I don't see any reason why it has to wait for that? This is for hooks only and has nothing to do with the generic class.
Comment #5
sunI understand. However, this has a low priority currently and the patch affects many lines in mollom_test.module, possibly conflicting with all the REST/class changes, and a big part of the existing mollom_test.module will be moved into a separate mollom_test_server.module.
If time permits, I'll try to commit this change before the next release that's planned to happen in the next days. Overall, however, the plan is to only commit absolutely required changes until the REST/class patch/branch lands, as pretty much every single patch needs to be merged manually into the 7.x-class branch.
Comment #6
sunActually, I don't really see why mollom_test module has to move its implementations -- we don't need to test Drupal core functionality.
So here's a revised patch that simply adds hook_hook_info().
Comment #7
sunAdded docs for the added hook_mollom_form_list_alter().
Comment #8
dries commentedMmm, hook_hook_info() is normally used to expose a list of triggers that users can assign actions to. Based on the issue description, it sounds like this hook_hook_info() is supposed to do something else?
Comment #9
sunThis core hook was re-purposed in D7. The original purpose in D6 was very poorly named and only bound to the actions system.
hook_hook_info() allows modules that "own" a hook to specify that other modules may put the hook implementations into include files that are loaded on demand.
Comment #10
sunSince this is a best practice for contributed modules in D7, I went ahead and committed this patch to master.
To clarify once more:
hook_hook_info() is in particular useful when a contributed module implements optional support for another contributed module.
This means, if the other module is not installed, then a site would load "dead" support code for another module on every single request, even though the code is and will never be executed.
An actual example would be users of Webform module that do not use Mollom [yet ;)]. Webform currently has to load the support code for Mollom on every site, regardless of whether Mollom is enabled. Now it is able to offload that code into webform.mollom.inc.