Split up mollom.module to mollom.*.inc
Freso - August 16, 2008 - 09:57
| Project: | Mollom |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | task |
| Priority: | normal |
| Assigned: | Dave Reid |
| Status: | closed |
Description
Mollom has a bunch of functions that are only called certain places (not just admin stuff). Let's split them up and use the awesome 'file' => 'mollom.*.inc' now that it's available!
#1
Agreed. I will look into splitting at least an admin.inc out. Not sure if a pages.inc is needed.
#2
Patch ready for review. The only other function able to be split out into the mollom.admin.inc file was _mollom_verify_keys().
#3
There are eight functions (a little more than 150 lines) that are possible to take out to a mollom.pages.inc, for the various mollom/* callbacks. I think that's enough reason to do this split as well.
#4
And all (563) tests pass with the patch from #2.
#5
Fixed a very minor coding style change in mollom_report_contact() in mollom.pages.inc. Confirmed all tests pass and manually reviewed the CAPTCHAs work and the report pages work as well.
#6
Rerolled with coder patch landing.
#7
I don't think this is important. I'd prefer to keep everything in a single file for the time being. There is no real performance advantage to splitting things up, and it makes it more difficult to develop.
#8
Re-rolled to split out a few functions into mollom.inc that don't need to be loaded on every full bootstrap.
#9
I'd really like this to be re-considered. Performance was the primary reason we can split modules into .admin.inc and .pages.inc in Drupal 6. There's no need to have the admin settings code (a fairly largish function) loaded on every single page request, and the same thing with all the report node/comment/mail code. After working with D6 for so long, having a larger module file is harder for me to scan. Any developer on an IDE with code completion shouldn't have any problems dealing with separate files. But that's still a personal preference. The main reason for this is its-the-way-Drupal-core-does-it. Looking forward to Drupal 7 and it's higher memory usage, splitting module files will be even more important.
#10
I'd also very much like this to land before any work on the UI refresh as part of the any-forms patch. Having a mollom.admin.inc will make things a *lot* easier since there will be a lot more code added.
#11
Committed to CVS. Thanks Freso for helping!
#12
Automatically closed -- issue fixed for 2 weeks with no activity.