Move more token filters to analysis-common module #25580
Conversation
martijnvg
added
:Search Relevance/Analysis
cbuescher
left a comment
cbuescher
left a comment
There was a problem hiding this comment.
LGTM, I left a question about the new yaml test just out of curiosity.
There was a problem hiding this comment.
I'm curious why you changed this from calling AnalysisTestsHelper to a private helper method. Is it because the test is in the module now and cannot depend on core-test anymore?
There was a problem hiding this comment.
Nevermind my previous comment, I see AnalysisTestsHelper is accessible here.
There was a problem hiding this comment.
yes. it is a tiny helper method to reduce the line length where it is being used. (because also including new CommonAnalysisPlugin()) would in many cases cross the 140 character limit)
There was a problem hiding this comment.
Just curious: are these new test because I cannot find a corresponding file where they might be moved from?
There was a problem hiding this comment.
yes, these rest tests are new. It is the only we can be sure we wired up the factory correctly and the token filter is actually working.
The following token filters were moved: common grams, limit token, pattern capture and pattern raplace. Relates to elastic#23658
martijnvg
force-pushed
the
move_more_token_filters_to_analyis-commons_module3
branch
from
be77767 to
6db708e
Compare
3 participants
Move the
common_grams,limit,pattern_captureandpattern_replacetoken filter to analysis-common module.Relates to #23658