Make ICU tokenizer the default #2707
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Also applies to Path.write_text().
|
I'm happy to get Ubuntu-22 merge-ready again later, it's not pressing. Just something I know users eventually will ask for. We've been using ICU tokenizer fine for months, too, no issues. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The ICU tokenizer has been used in production for half a year on osm.org without major issues. Time to make it the default tokenizer. With the legacy tokenizer gone, there is also no need to automatically build the PostgreSQL module any longer. The code will remain and is still tested on the CI but the compilation needs to be explicitly activated. Existing installations should have the module around already, so there is very little need in the future to compile it.
@mtmail I'm aware that this conflicts rather strongly with the ongoing work in #2691. I'm happy to wait for it to be merged and resolve the conflicts here afterwards. But there are a couple of simplifications in the CI scripts that might make the other PR simpler as well. Among others, I have reduced the former 'icu-test', which now has become 'legacy-test' to be run on 20.04 only. Given that it is the past, it will be enough.