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.

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

move CSVUtil to common from analyzer nori and kuromoji #12390

Merged
merged 1 commit into from
Nov 2, 2023

Conversation

twosom
Copy link
Contributor

@twosom twosom commented Jun 23, 2023

Description

This PR moves the CSVUtil class that existed separately in the Analyzer Nori and Kuromoji to the module :analysis:common.
By consolidating the CSVUtil class, we can prevent duplication and improve maintainability across the Analyzer Nori and Kuromoji.

Closes #12389

Copy link
Contributor

@stefanvodita stefanvodita left a comment

Choose a reason for hiding this comment

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

The change looks reasonable to me. I like the two small improvements in CSVUtil.

Copy link
Member

@mikemccand mikemccand left a comment

Choose a reason for hiding this comment

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

Looks great, thanks @twosom! A nice refactoring to combine two forked impls ... I'll merge!

@mikemccand mikemccand merged commit 30db217 into apache:main Nov 2, 2023
5 checks passed
@mikemccand
Copy link
Member

I merged to main but there are quite a few conflicts on backport to 9.x -- any chance you could open a backport PR @twosom? Thanks!

rmuir pushed a commit to rmuir/lucene that referenced this pull request Nov 2, 2023
@twosom
Copy link
Contributor Author

twosom commented Nov 6, 2023

I merged to main but there are quite a few conflicts on backport to 9.x -- any chance you could open a backport PR @twosom? Thanks!

OK! I'll open backport PR ASAP! Thanks!

javanna added a commit to elastic/elasticsearch that referenced this pull request Nov 9, 2023
IndexDiskUsageAnalyzer and IndexDiskUsageAnalyzerTests, as well as CompletionFieldMapper, CompletionFieldMapperTests and CompletionStatsCacheTests need adjusting after apache/lucene#12741 , to refer to the latest postings format.
KuromojiTokenizerFactory needs adjusting after apache/lucene#12390
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Need to resolve the duplicate CSVUtil classes in analyzer Nori and Kuromoji
3 participants