FeedAPI module is a monolithic piece of 1600 lines of code. Much of it could live in a seperate file that is only loaded if required.

Off the top of my head:

feedapi.module - Entry point for Drupal
feedapi.inc - aggregation processing
feedapi.admin.inc - admin pages
feedapi.opml.inc - OPML imort/export

CommentFileSizeAuthor
#2 feedapi_split_feed_processing.patch30.09 KBaron novak

Comments

aron novak’s picture

Well, while I started to implement this, I realized that this step would be equal with breaking 3rd party feedapi modules :)
I'd vote only for feedapi.admin.inc and feedapi.opml.inc .

function _feedapi_mapper_get_items_merged($node) {
  if ($node->feed) {
    $feed = feedapi_parse($node->feed);

Logically feedapi_parse should live under the feedapi.inc. But in Drupal 6 we do not have that elegant drupal_function_exists() function, this means that 3rd party modules should be aware of including the right files just in time.
In Drupal core, the explosion of aggregator into tons of .inc files is only started from Drupal 7, there is a good reason for that :)

aron novak’s picture

StatusFileSize
new30.09 KB

The first part (opml / admin.inc) is committed.
For the others, here is a patch.
The new feedapi.inc is almost 400 lines long :)

alex_b’s picture

Looking good, I like how this module starts to shrink.

1) Shouldn't _feedapi_sanitize_processors() go to feedapi.inc ?
2) _feedapi_get_stat() should not be in feedapi.inc but in feedapi.admin.inc, right?
3) Should we move the internals of feedapi_form_alter() into _feedapi_content_type_form_alter() in feedapi.admin.inc and a _feedapi_node_form_alter() in feedapi.inc ? feedapi_content_type_validate() and feedapi_content_type_submit() could move with it.

avpaderno’s picture

To move the internals of feedapi_form_alter() in feedapi.admin.inc would not help much.
It makes sense to pass the code to an external file only when Drupal automatically includes that file; if the module must manually include the file, then it should be better to keep the code in the main file (generally speaking).

aron novak’s picture

Status: Active » Closed (fixed)

done already