Skip to content
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.

Sign up for GitHub

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

Jump to bottom

LUCENE-8682: remove deprecated WordDelimiterFilter[Factory] classes #202

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

LUCENE-8682: remove deprecated WordDelimiterFilter[Factory] classes #202

wants to merge 6 commits into from

Conversation

cpoerschke
Copy link
Contributor

@cpoerschke cpoerschke commented Jul 2, 2021 •
edited by mocobeta
Loading

@cpoerschke
Copy link
Contributor Author

And I'd like to mention that https://github.com/apache/solr/search?q=WordDelimiterFilter currently still contains references and we may wish to deal with them before merging this pull request or perhaps afterwards?

@cpoerschke
Copy link
Contributor Author

And I'd like to mention that https://github.com/apache/solr/search?q=WordDelimiterFilter currently still contains references and we may wish to deal with them before merging this pull request or perhaps afterwards?

Ah, there's already https://issues.apache.org/jira/browse/LUCENE-8682 with a patch which includes Solr.

@cpoerschke cpoerschke changed the title LUCENE-8638: remove deprecated WordDelimiterFilter[Factory] classes LUCENE-8638, LUCENE-8682: remove deprecated WordDelimiterFilter[Factory] classes Jul 2, 2021
@cpoerschke cpoerschke marked this pull request as ready for review July 2, 2021 19:27
@cpoerschke cpoerschke changed the title LUCENE-8638, LUCENE-8682: remove deprecated WordDelimiterFilter[Factory] classes LUCENE-8682: remove deprecated WordDelimiterFilter[Factory] classes Jul 3, 2021
@cpoerschke
Copy link
Contributor Author

And I'd like to mention that https://github.com/apache/solr/search?q=WordDelimiterFilter currently still contains references and we may wish to deal with them before merging this pull request or perhaps afterwards?

Ah, there's already https://issues.apache.org/jira/browse/LUCENE-8682 with a patch which includes Solr.

apache/solr#200 created with the LUCENE-8682 patch as its starting point.

msokolov
msokolov reviewed Jul 10, 2021
View reviewed changes
Copy link
Contributor

@msokolov msokolov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[copied from JIRA]

I may be confused here, but I don't think WordDelimiterGraphFilter is a full replacement for WordDelimiterFilter since it can't be used in conjunction with other filters that consume or produce graphs, like SynonymGraphFilter. The same comment applies to SynonymFilter. If we remove these, I think we need to fix all the -Graph filters so they can consume filter graphs? Or maybe that already happened?

@cpoerschke
Copy link
Contributor Author

Thanks @msokolov for copying across the comments from the JIRA! I'll convert the pull request to "draft" to indicate the status of the deprecation process.

@cpoerschke cpoerschke marked this pull request as draft July 10, 2021 21:52
@cpoerschke cpoerschke mentioned this pull request Jul 10, 2021
Draft
@asfimport asfimport mentioned this pull request Dec 8, 2021
Open
@mikemccand
Copy link
Member

but I don't think WordDelimiterGraphFilter is a full replacement for WordDelimiterFilter since it can't be used in conjunction with other filters that consume or produce graphs, like SynonymGraphFilter.

Yeah I'm pretty sure this is still the case, unfortunately. It's a hard problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@msokolov msokolov msokolov left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@cpoerschke @mikemccand @msokolov