Skip to content

Conversation

@gpaimla
Copy link
Contributor

@gpaimla gpaimla commented Nov 15, 2019

This PR adds a new analyzer and stemmer for the Estonian language.
Closes #48895

@gpaimla
Copy link
Contributor Author

gpaimla commented Nov 15, 2019

@elasticcla refresh

@cbuescher cbuescher added the :Search Relevance/Analysis How text is split into tokens label Nov 15, 2019
@elasticmachine
Copy link
Collaborator

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

@cbuescher cbuescher self-assigned this Nov 18, 2019
Copy link
Member

@cbuescher cbuescher left a comment

Choose a reason for hiding this comment

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

Hi @gpaimla, thanks a lot for your contribution and adding this Lucene analyzer to ES. The change looks great to me in general. I have just one minor ask. Could you add the new analyzer to the documentation page for language analyzers over at docs/reference/analysis/analyzers/lang-analyzer.asciidoc and maybe add the stop word filter to docs/reference/analysis/tokenfilters/stop-tokenfilter.asciidoc?
For the language analyzer page please adapt one of the existing sections about how to reimplement the language analyzer for the case that users need to customize them.
Thanks again, looking forward for these changes after which I will run our CI test suite on the change.

@gpaimla gpaimla requested a review from cbuescher November 18, 2019 14:19
Copy link
Member

@cbuescher cbuescher left a comment

Choose a reason for hiding this comment

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

Great, thanks for the update. I will run the CI tests and merge if they pass or get back to you if the PR needs some more changes (which I doubt).

@cbuescher
Copy link
Member

@elasticmachine test this please

@cbuescher cbuescher merged commit d1ea991 into elastic:master Nov 18, 2019
cbuescher pushed a commit that referenced this pull request Nov 18, 2019
This PR adds a new analyzer and stemmer for the Estonian language.

Closes #48895
@cbuescher
Copy link
Member

Thanks again @gpaimla for working on this. I merged the PR to master and our 7.x branch which should be used for the 7.6 release.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[feature request] Implement Lucene Estonian analyzer in Elasticsearch

5 participants