OK, let's try one of the big ones. This patch slides system.module nearly clean in half. That's a savings of about 1600 lines. I also added a few more comments to functions, but otherwise no functional changes.

Let's see if this can get in before some other patch breaks it horribly. :-)

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Dries’s picture

Looks a little funny already:

 $ patch -p0 < f.patch 
The next patch would create the file modules/system/system.admin.inc,
which already exists!  Assume -R? [n]
Crell’s picture

FileSize
130.67 KB

D'Oh! That's because system.module is the only module that already had a page file, so I don't need to add it to the Entries file. Take 2!

Crell’s picture

FileSize
129.89 KB

Rerolling to keep up with head.

Crell’s picture

FileSize
159.43 KB

Rerolling, and this version also splits off theme functions as well per conversation with merlinofchaos and pwolanin. Total savings, over 50%!

pwolanin’s picture

Status: Needs review » Reviewed & tested by the community

Looks good , once I figured out how to get a menu rebuild to happen (edit & save a content type, if you're not using Devel module).

All pages and functions work as expected.

Dries’s picture

Looks like this patch needs a quick re-roll.

Crell’s picture

Status: Reviewed & tested by the community » Needs work

CNW until I have a chance to reroll it for the block caching patch...

Crell’s picture

Status: Needs work » Needs review
FileSize
161.34 KB

Another advantage of splitting Drupal up into smaller files is that there's hopefully less chance of patches bumping into each other, at least as far as CVS is concerned. *sigh*

Caleb G2’s picture

Status: Needs review » Needs work

I guess there's a larger problem here. Patch #25 does the same thing the patch I tried to re-roll did (never posted it since it didn't work) -- after applying the patch and running the installer Drupal gives WSOD.

pwolanin’s picture

@CalebG - Are you installing form scratch, or using an existing install? If the latter, did you rebuild your {menu_router}?

Caleb G2’s picture

pwolanin: From scratch (fresh check out of head)

Crell’s picture

CalebG: Did you apply the patch before you ran the install or after? If after, you will need to put menu_rebuild() in your index.php right before the menu_execute_active_item() call and load the main page once to refresh the menu.

Caleb G2’s picture

Sheesh guys - come on! What kind of goofball do you think I am?!

Of course, I applied the patch beforehand. :)

Crell’s picture

Status: Needs work » Needs review
FileSize
161.02 KB

Sorry, Caleb, I was reading quickly and didn't quite realize the illogic of my question. :-)

Found the problem, though. The installer uses _system_zonelist(), which had previously been moved to the admin pages file. Attached version doesn't move it. Of course, that means it's not strictly an internal function and should not be an underscore function, but I'm not going to worry about that in this patch. :-)

asimmonds’s picture

Status: Needs review » Needs work

A fresh install from scratch seems to work fine now.

Going to admin/build/themes, generates a error that the theme function 'theme_themes_form()' can not be found, should this be theme_system_themes_form() ?

Crell’s picture

Status: Needs work » Needs review
FileSize
161.03 KB

Why yes. Yes it should. :-) Rerolled, fixes #15.

pwolanin’s picture

Status: Needs review » Needs work

going to the module uninstall page I get this error:

warning: call_user_func_array() [function.call-user-func-array]: First argumented is expected to be a valid callback, 'theme_system_modules_uininstall' was given in /www/drupal6/includes/theme.inc on line 505.
pwolanin’s picture

looks like a typo in system_theme()

pwolanin’s picture

maybe we should get this one in ASAP: http://drupal.org/node/168028 so we can alleviate the need to put in all the function names...

pwolanin’s picture

Status: Needs work » Needs review
FileSize
160.61 KB

Here's a re-roll taking advantage of the fact that the patch I linked above was committed.

Now all the theme functions seem to work.

Dries’s picture

Status: Needs review » Fixed

Tested, seems to work. Thanks.

Anonymous’s picture

Status: Fixed » Closed (fixed)