Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.The more I think about it, the less sure I am that we are currently doing this the right way. The Search API is a generic framework, and none of the Facet API-specific code it currently contains really needs to be directly in it, instead of in a custom module.
Putting this in its own module would:
- Solve the depency more nicely.
- Make the code cleaner again (no mix between framework and Facet API integration).
- Remove (probably minor) performance drawbacks for people not using the Facet API.
- Demonstrate that such major improvements are possible from contrib modules, like the current Facets module does.
Does any of you disagree with this, or have some counter-arguments?
| Comment | File | Size | Author |
|---|---|---|---|
| #6 | 1231570--move-to-own-module-6.patch | 65.42 KB | drunken monkey |
| #2 | 1231570--move-to-own-module-2.patch | 65.38 KB | drunken monkey |
Comments
Comment #1
drunken monkeyNew module would probably go into contrib/search_api_facetapi.
Or would it maybe even be better to replace the old Facets module with this? (OK, probably not, but it would have its advantages …)
Comment #2
drunken monkeyThis would make the change, reviews would be appreciated.
There is already code committed for the search_api_facets module taking this into account, and a rough skeleton for the search_api_facetapi module.
Comment #3
cpliakas CreditAttribution: cpliakas commentedThe overall approach makes sense to me. Will download and test.
Comment #4
drunken monkeyPlease don't, yet. Sorry for posting it in this state, I didn't test it at all. Now there are the silliest mistakes, like wrong menu definitions, etc.
Comment #5
drunken monkeyComment #6
drunken monkeyOK, this is much more like it.
Sorry again for the untested previous patch!
Comment #7
drunken monkeyAs everything seems to work: committed.