I don't see why this should be loading the library in hook_boot. If something needs it then it can run then. Also, it seems nice to use a static to know if it's loaded or not.

CommentFileSizeAuthor
querypath_dont_always_load.patch979 bytesgreggles

Comments

mbutcher’s picture

mbutcher commented

I've been working on this for the 3.x branch, where it will use an autoloader instead. But now I'm thinking about adding this to the 2.x branch anyway. It's an easy enough fix.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

autoloader seems fun. I've now actually tested this and it seems to work well. Anything blocking it from inclusion?

mbutcher’s picture

mbutcher commented

I think we should go ahead and add this to the 2.x branch. Since I'm really short on time at the moment, I've given you permissions to commit and to make the release. You are, of course, free to make other changes. I think we're totally on the same page about what a library module like this ought to do.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented
Title: it's not always needed in hook_boot » QueryPath is not always needed in hook_boot
Status: Needs review » Fixed

Thanks, fixed - http://drupalcode.org/project/querypath.git/commit/ca5c928

I removed the static variable thing. I feel like that was a premature optimization and that php's require_once probably can do that faster/lighter than a static.

In terms of "what a library module like this should do" my sense is "nearly nothing" so I guess that works for you :)

jacobpov’s picture

jacobpov commented

doesn't work with feeds , when I try to click start it shows a blank page now with the latest dev.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

@jacobpov - thanks for reporting that! I've opened an issue at #1801420: Update to latest QueryPath code to track it.

@mbutcher - any idea which other modules on drupal.org use QueryPath?

I think we should wait a pretty long time before making the next release just so people can upgrade. We could also consider changing this so that hook_boot is still implemented and calls querypath_include_code and does a watchdog along the way so that people know to update their code.

Status: Fixed » Closed (fixed)

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

ludo.r’s picture

ludo.r
Location Brussels
commented

What changes should be made in modules that use QueryPath?
module_load_include() ?

Here's the project : http://drupal.org/project/querypath_filter

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

@dolu - I created #1817592: Update to latest querypath style of inclusion. #1801420: Update to latest QueryPath code might also be instructional.

We plan to make the release in the next 2 weeks or so. It would be great if you could make a new release by then.

mkalkbrenner’s picture

mkalkbrenner
Primary language German
Location 🇩🇪
commented

You cause a lot of trouble ;-)
Came from #2007906: PHP Fatal error: Call to undefined function qp() in apachesolr_confgen.module on line 77

Nevertheless I really appreciate that change because I have to use querypath 2 and 3 in parallel (on different pages of a site).

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

@mkalkbrenner, I opened a half dozen or so issues to try to proactively alert people to the problem. Sorry I missed that one :(