-
Notifications
You must be signed in to change notification settings - Fork 199
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WIP: migrate search engine to full-text-search package (#748) #968
base: master
Are you sure you want to change the base?
Conversation
Would this let us implement #727 as well? |
It gives one possible strategy, yes. The We'd then need to expose a query API wrapping |
I think we really should merge this. Sorry for letting it languish. Can you explain what "The queryExplain API apparently changed fractionally, which affects searchPackagesExplain." means in more detail? (the memsize issue doesn't matter so much, imho) |
No need to apologise, because this clearly wasn't quite in a mergeable state when I threw it together, and apologies myself for not getting back to this sooner. I've looked slightly more closely at the Also, I noticed that |
@adamgundry Since the If you think it is better to keep the unreferenced code around, I wouldn't mind too much. I just think it is better to err on the side of removal. To me it is a philosophical choice, one could go either way, but if we choose to keep unused code around, that choice should be explicit so people can take it into consideration. |
See #748. This removes the search engine code in favour of the
full-text-search
package (which was derived from the Hackage codebase originally, so is almost exactly compatible). This is a very quick attempt; I haven't verified if there are code changes on either side that need to be dealt with.It has a couple of awkward places that still need attention:
queryExplain
API apparently changed fractionally, which affectssearchPackagesExplain
.MemSize
instance, becausefull-text-search
does not expose the internal representation ofSearchEngine
.