Skip to content

Fix split package in analysis-icu plugin#78037

Merged
rjernst merged 3 commits intoelastic:masterfrom
rjernst:split-packages/icu
Sep 21, 2021
Merged

Fix split package in analysis-icu plugin#78037
rjernst merged 3 commits intoelastic:masterfrom
rjernst:split-packages/icu

Conversation

@rjernst
Copy link
Copy Markdown
Member

@rjernst rjernst commented Sep 20, 2021

The analysis-icu plugin reuses server package names for analysis and
mappers. This commit moves the plugin implementation to use a single
package name, o.e.p.analysis.icu

The analysis-icu plugin reuses server package names for analysis and
mappers. This commit moves the plugin implementation to use a single
package name, o.e.p.analysis.icu
@rjernst rjernst requested a review from romseygeek September 20, 2021 20:07
@elasticmachine elasticmachine added the Team:Search Meta label for search team label Sep 20, 2021
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-search (Team:Search)

Copy link
Copy Markdown
Contributor

@romseygeek romseygeek left a comment

Choose a reason for hiding this comment

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

LGTM, I left some questions around license headers.

We don't really have a good deprecation route for analysis plugins (it's pretty messy deprecating built-in analysis components as well). It would be nice to be able to cleanly say 'these components are deprecated' in the plugin definition. Something to add to the pile.

@@ -1,4 +1,12 @@
package org.elasticsearch.index.analysis;
/*
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is a directly copy of lucene's old ICUCollationKeyFilter I think? In which case it should just keep the Apache 2 license header AIUI.

@@ -1,4 +1,12 @@
package org.elasticsearch.index.analysis;
/*
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same comment as above

@@ -1,4 +1,12 @@
package org.elasticsearch.index.analysis;
/*
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

And here

@rjernst rjernst merged commit a725d28 into elastic:master Sep 21, 2021
@rjernst rjernst deleted the split-packages/icu branch September 21, 2021 17:40
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

💔 Backport failed

Status Branch Result
7.x Commit could not be cherrypicked due to conflicts

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 78037

@ChrisHegarty ChrisHegarty mentioned this pull request Sep 22, 2021
58 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>non-issue :Search Relevance/Analysis How text is split into tokens Team:Search Meta label for search team v7.16.0 v8.0.0-beta1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants