Facet API has implemented hook_hook_info() since RC1, and it allows for autoloading of *.facetapi.inc files containing Facet API hook implementations. Moving the hooks to a separate *.facetapi.inc file will reduce the module's footprint by roughly 180 lines of code on non-search pages.

Currently less than 13% of users run on non-RC Facet API releases, so the chances of someone running the latest version of Apache Sole Search Integration with the proposed change integrated and a version of Facet API that doesn't implement hook_hook_info() is incredibly small, and probably non-existent.

CommentFileSizeAuthor
#1 facetapi-inc-1592778-1.patch12.28 KBcpliakas

Comments

cpliakas’s picture

cpliakas commented
Status: Active » Needs review
StatusFileSize
new facetapi-inc-1592778-1.patch12.28 KB

The attached patch makes the proposed change.

pwolanin’s picture

pwolanin commented
Status: Needs review » Needs work

No - it's probably better not to do this since it will make 6.x-3.x backporting harder.

cpliakas’s picture

cpliakas commented

Really? I'm not seeing how, other than not being able to cleanly apply a patch between one version and another. Even if we didn't move the Facet API hooks to a separate file, the differences between CCK / Field API and DPTNG / 1999 style database queries make the modules significantly different anyways. To me reducing the module's footprint, even if it is seemingly small, is worth that trade-off.

Just my $0.02,
Chris

pwolanin’s picture

pwolanin commented

Right, I was just thinking about patches and code organization diverging.

The amount of footprint saved here doesn't seem worth any extra confusion?

pwolanin’s picture

pwolanin commented

Right, I was just thinking about patches and code organization diverging.

The amount of footprint saved here doesn't seem worth any extra confusion?

cpliakas’s picture

cpliakas commented

The amount of footprint saved here doesn't seem worth any extra confusion?

I personally don't feel like it would add confusion since it would actually better segment the logic, however I am not spending as much time in the code as you guys, so I will happily yield to whether you think this will be too much of a hinderance. If so, feel free to close as "won't fix".

Thanks,
Chris

pwolanin’s picture

pwolanin commented
Status: Needs work » Closed (won't fix)

For now at least - especially as 6.x-3.x backports are lagging, let's minimize any file differences.

cpliakas’s picture

cpliakas commented

Gotcha. Thanks for your attention to this.

cpliakas’s picture

cpliakas commented
Issue summary: View changes

Updated issue summary.