require_once(drupal_get_path('module', 'activity') ."/contrib/{$module}/{$module}.module");
is pretty bad. Makes upgrading annoying (as I can't just remove the directory for a new version, I have to carefully pick out any extra modules I may have added), and development painful. Note that Drupal always knows the path to a module, regardless of whether it is installed or not - you can run drupal_get_path('module', 'favoritenodesactivity') just fine. This would immediately solve my annoyance, and open the path to easier third party development.
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | activity.module.355633.patch | 3.27 KB | jaydub |
Comments
Comment #1
morbus iffAnother annoyance:
which makes it impossible for a properly named module to implement your hooks. I should be able to have a module called "mymodule.module" that responds to your hook, without forcing me to create yet another module called mymoduleactivity.
Comment #2
jaydub commentedOk I replaced the use of require_once in the original issue with the d6 function module_load_include() which should be more flexible.
Will come back and address #1 later.
Comment #3
morbus iff#1 should be ripped out entirely. My feeling is:
* rip out the "list all the settings at the bottom of the Activity Settings page"
* force each module to define their own menu underneath the primary settings, as they should.
The current functionality surrounding #1 is just plain ol' bad design.
Comment #4
morbus iffAs for module_load_include(), that's not the right answer. It works, from a technical standpoint, but it's a direct abuse and overloading of the function. The function is focused around loading /include/ files, not the actual .module file itself. Just because it CAN, due to the way it has been programmed, doesn't mean you should ignore the /intent/ behind its documentation and purpose.
Unless you can find an instance in core that abuses the function this way, please change it back to include_once and drupal_get_path.
Comment #5
lelizondo commented#359209: require_once() Bug is related to this issue.. I'm using 5.x-4.x.. I think this should be critical since I can't install because of:
Comment #6
sirkitree commentedComment #7
jaydub commentedGoing back to #1, this patch removes the 'nice name' code in favor of something that doesn't depend on the module being named XXXXactivity.module.
Still think it's ok to have the settings menu item for each module be generated in activity.module rather than in each contrib.
Comment #8
sirkitree commentedThe patch is fine, but really doesn't address the underlying issue here, which is that we're not using the hook system correctly at all. We're forcing modules to have a particular naming conventions in order to comply to our wishes, instead of just providing a hook definition that ANY module can implement. This is actually what led to some of our headaches in having to maintain these sub-modules in the first place.
The next version of this module will not have any of these limitations and we'll be making better use of drupal_alter(), module_implements(), and module_invoke_all(). Let's postpone this one as it's a pretty major undertaking.
Comment #9
sirkitree commentedI'm doing a little preliminary work here and trying to get feedback on the next version. Posting a link for reference.
http://groups.drupal.org/node/19249#comment-67046
Comment #10
sirkitree commentedclosing. 1.x no longer supported.