Namespacing for Sub-modules

Yes, somewhere there is an issue to properly name space the spam filter modules -- this will be happening after the 6.x-1.0 stable release (which should be next week). This issue should be marked as a duplicate of that issue -- and a link to that issue should be placed here.

Yes. None of the issues that mention this actually have this as their primary purpose.

Right now, it's looking like this will wait until after the final 1.0 release.

In the development snapshot, the sub-modules are still incorrectly named; the short name of those modules should start with spam_.
This is also true for version 6.x-1.0.

The long name of a module is the same of another module (Custom filter); to avoid confusions, it should be renamed.

ok, I've added a schema upgrade function for each of the filters that require it (surbl has no db variables).

Since most files have been renamed, a patch isn't going to work for this, should I just dump a tar.gz in the comments here?

See also #648846: Conflict between some functions and possible hook implementations.

I agree with Jeremy on the patch file vs. a gzip.

The naming inconsistencies do indeed need fixing -- Jeremy and I talked about it, and I think the best move is to have spam_filter_ and spam_content_ name prefixes, with the name of the submodule following that. This:
1) maintains the "spam" prefix (and thus the Drupal-wide submodule naming "standard") for all contributed modules;
2) allows for clarity in the hierarchy for the two fundamentally different submodule types we support;
3) avoids the potential conflict with hook_filter from having modules end with the word "filter".

So, as an example, to fix the specific modules you mentioned:
"There are a few functions that were inconsitently named, like custom_spam_filter() and custom_spam_custom_operations() (!). Should these be renamed spam_custom_filter() or spam_custom_spam_filter()? Should custom_spam_custom_operations() be reduced to spam_custom_operations()?"

custom_spam_filter()
should be:
spam_filter_custom_spam()
- as it would be part of the "spam_filter_custom" submodule

custom_spam_custom_operations()
should, as part of the same submodule, be:
spam_filter_custom_operations()
- unless spam_filter_custom has more than one "operations()" function, which I don't think it does off the top of my head but can't remember for sure -- if I'm wrong about that, and the second "custom" specifier is necessary for clarity, the function would then be "spam_filter_custom_operations_custom()" or "..._custom_operations()" or whatever at that point.

Jeremy and I discussed abbreviations, as those can get a little long, but he noted (correctly, I think) that Dries tends to prefer avoiding abbreviations where possible. I suppose I could perhaps be talked into just abbreviating the second part of the name -- say, spam_flt_custom... or spam_cnt_node... instead of spam_filter_ and spam_custom_, but I would be pretty against anything at the beginning of the name that wasn't "spam." That's a pretty standard thing, I think.

ok, I'll post a patch. This is going to take a bit more work now though, since I was just namespacing the filter modules before.

Man, if you even just do the filter modules alone, I'm sure we'd really appreciate it. I know I've been swamped here at the end of the year, and this was a particularly high priority item for me, so thank you VERY much for your effort on this.

So basically filter modules like custom.module should be renamed to spam_filter_custom.module, and all hooks renamed respectively?

Yes. (Unless somebody feels strongly about using spam_flt_ or something. And by somebody, I mean Jeremy -- I think he may be the only other person that could convince me to shorten those...)

Why spam_content_blah? Why wouldn't it be fine to leave it as spam_blah? If you really want a separate namespace wouldn't spam_core_blah be more appropriate?

Well, you get to the answer of the first part there -- it's because I thought a separate namespace for them would be appropriate. Since we have to make a separate one for spam_filter anyway, it seems the appropriate and consistent thing to do to create one for content filters as well. (Who knows -- maybe someone will make a custom Drupal content type they want to call "filter"...) :) Since we're going to be renaming so much stuff anyway, it seems like it would be the right time to just do it "right" and end up with a consistent product to build from in the future.

As far as the second part -- spam_core_nnn instead of spam_content_nnn? I think the module core is the module core, and that's defined by spam_nnn. The content filters are specifically to handle different content types. If someone were to contribute another content filter, we'd have a third-party contribution called spam_core_nnn -- doesn't that imply that's part of the main module, and not a third-party contrib at all? I think spam_content_nnn makes more sense.

With custom_spam_filter() --> spam_filter_custom_spam(), what does the trailing _spam mean? Might it be better to make it more explicit, like spam_filter_custom_spamfilter() or something?

Y'know, that might just be a mistake on my part. The only reason I added the _spam to the end was because I thought there was a hook_spam in there somewhere that was launching -- but that isn't what that function is for, is it? Slip of the brain, I think. My fault for not actually referring to the code when writing those explanations. My main point was the spam_filter_custom_ prefix; after that, the function should just be named after what it does unless it's specifically calling an API hook. So there's leeway there for the patch writer's discretion, I think.

Maybe it would be a good idea to list the hooks that exist, and the names we're proposing to change them to, before making any actual patches. I'm now a little confused... ;)

Here they all are...

Ok, fixed that problem, didn't realise that comment_spamapi was actually adding the spamapi to the core comment module. nifty.

This seems to work, I've tested most things, but it definitely needs a bit of a look over. I HAVEN'T done anything outside of the scope of this bug that's mentioned in this bug - just namespacing, and editing stuff to account for the new name spacing. The problems with the hook names isn't fixed, and there are still a few functions that have odd names, and I haven't documented anything. I would propose getting this in, then doing those things, otherwise it's gonna get over-complicated.

I've never created a patch with new files before, so I can re-do it if this isn't appropriate:
diff -uprN /home/files/coding/drupal/spam sites/all/modules/spam/ > /home/files/coding/drupal/spam_namespacing.patch

No it doesn't: the spam.module file only has a few changes. The patch HAS to included every line of each of the other files, because the files names have changed, and it doesn't know that they're the same file.

That said, looking at it again, I see that I forgot to remove a debugging line, and I accidentally diff'ed in a whole bunch of backup files. Here's the same again, but cleaned up.

The link you posted to is mostly about CVS diff'ing. Is that what you'd prefer?

all files like spam/filters/[modulename]/[modulename].(module|install|info) have been renamed spam/filters/spam_filter_[modulename]/spam_filter_[modulename].(module|install|info)

all files like spam/modules/spam_[coremodule].inc have been renamed spam/modules/spam_content_[coremodule].inc

That one's easy. modules/ directory moved to content/ (BTW, I meant "core modules" as in drupal core modules - node, comment, user, cause that's how the .inc files are named.)

This patch applies cleanly to the latest dev release with patch -E -p3 < spam_namespacing.patch

I would prefer it if we could just get this one in, and then work on the problems with hooks and internal documentation and so on...

Please? if we can get this in, then we can start working on the real bugs.

OK, tested -- neither patch applies cleanly. The first one throws temp files everywhere, and the second one deletes most everything in the directory without readding it to its new directory. Further, the second one has some substantive changes to the first, and both have a substantive change to spam_admin_feedback_form_submit which is unexplained. I can't believe that these were tested against a clean CVS version of the code.

I'm trying to merge the differences in both patch files and clean them up from there, but hand-editing two 10k line patch files is pretty much a nightmare. Contacted naught101 in the hopes I can get a tarball from him and roll the patch myself; hoping to hear back soon.

Thank you, and sorry for the patch hassle. Next time I'll use CVS. Now, let's see if I can break it...

Basically runs but without simpeltest we can't be sure and I don't dare to set this to fixed. Hope I find some time.

OK, crud. Changed my mind. We're most of the way there, I think. Part of the idea was making the dir structure more "standard" for Drupal, and part was doc'ing some of the API as we went along... I'll keep putzing with this.

I'm at a point in Drupal now since I first subscribed to this issue that I am able to apply patches but for the patch above, I am wondering what files I need to apply this against?

I just realised that the 6.x-1.0 release doesn't have this in it. I thought it was supposed to be a bit of a blocker for 1.0 release?

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