The *.inc files in project* are weirdly organized, and useless in terms of conditionally loading the code we need. Now that we're ported to D6 (and the dust has mostly settled), we can make use of the D6 menu system to do a proper page split into sane .inc files. I'd like to do this before the 6.x-1.0 release. It'd probably also solve #376377: Issue + organic group incompatibility.

CommentFileSizeAuthor
#4 390856-4.project_usage_split.patch68.58 KBdww
#1 390856-1.project_usage_split.patch69.1 KBdww

Comments

dww’s picture

dww
we/he/they
commented
Assigned: Unassigned » dww
Status: Active » Needs review
StatusFileSize
new 390856-1.project_usage_split.patch69.1 KB

Let's do this in phases. Here's phase 0: project_usage. The one slightly wonky thing is that I moved 111 LoC into includes/date_api.inc for the API functions for manipulating dates. These are shared by both the page callbacks from pages.inc and project-usage-process.php. Seems silly to leave those in project_usage.module itself, since 99.999% of page loads on d.o don't need them, but having project-usage-process pull in all of pages.inc seemed silly, too. Any objections?

FYI: I installed on d6.d.o and cleared the cache and all seems to be working fine:
http://d6.drupal.org/project/usage/drupal
http://d6.drupal.org/project/usage/345833
http://d6.drupal.org/admin/project/project-usage-settings

drewish’s picture

drewish commented

project_usage_cache_time() seems like a good candidate for the date_api file.

is project_usage_project_page_link_alter() really split safe in usage/includes/pages.inc?

dww’s picture

dww
we/he/they
commented

project_usage_cache_time() seems like a good candidate for the date_api file.

Sure, that works. ;)

is project_usage_project_page_link_alter() really split safe in usage/includes/pages.inc?

No, that was a bug. It was buried among theme functions and I missed it. It belongs in the .module file. Now moved.

I also noticed that this is probably going to break date_rounding.test, but that's already probably broken since it needs to be ported to 6.x-2.x simpletest, etc. So, I'm just going to leave that for #405312: Port date_rounding.test to 6.x-2.* series of SimpleTest

dww’s picture

dww
we/he/they
commented
StatusFileSize
new 390856-4.project_usage_split.patch68.58 KB

Whoops, forgot to upload the current patch with the fixes from #2 and #3.

hunmonk’s picture

hunmonk commented
Status: Needs review » Reviewed & tested by the community

code split looks good. if it's been tested on d6.drupal.org, then it's ready to ship.

dww’s picture

dww
we/he/they
commented
Title: Do a proper page/code split » Do a proper page/code split of project
Assigned: dww » Unassigned
Status: Reviewed & tested by the community » Active

Committed to HEAD. Back to active for:
project
project_release
project_solr

project_issue lives at #405478: Do a proper page/code split of project_issue
cvs.module lives at #405484: Do a proper page/code split of cvs.module

dww’s picture

dww
we/he/they
commented
Category: task » bug

#408102: Call to undefined function project_project_nodeapi marked duplicate. Calling the current situation a bug given issues like this.

subcomandante’s picture

subcomandante commented

I have the same problem when I have another module http://drupal.org/project/menu_breadcrumb enabled.

It will be solved soon?

tnx

fuzzy_texan’s picture

fuzzy_texan commented
Issue tags: +6.x-1.0 blocker

Tagging for #439958: Document issues left before 6.x official release

mrfelton’s picture

mrfelton commented

@Subcomandante : see #454682: Issues with project and menu_breadcrumb integration. for more information on the menu_breadcrumb issue.

dww’s picture

dww
we/he/they
commented

Recently while on a plane, I did some work on a proper page split of project_release. Since it was so similar to what I did at #405478: Do a proper page/code split of project_issue I didn't bother posting these for review, I just committed:

http://drupal.org/cvs?commit=228132
http://drupal.org/cvs?commit=228134
http://drupal.org/cvs?commit=228138
http://drupal.org/cvs?commit=228140
http://drupal.org/cvs?commit=228142

Still more to do on this, but it's getting better...

subcomandante’s picture

subcomandante commented

After I reinstalled latest drupal + project + project issue, menu_breadcrumb seems working now.

drumm’s picture

drumm
he/him
Location NY, US
commented

Are there any specifics for what needs attention to complete this?