Deprecating levenstein in favor of levensHtein#27409
Deprecating levenstein in favor of levensHtein#27409DaveCTurner merged 4 commits intoelastic:masterfrom
levenstein in favor of levensHtein#27409Conversation
|
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually? |
|
Thanks @olcbean but I'm afraid it's a bit more complicated than this. This is a breaking change, so we need to support both spellings, and emit a deprecation warning for the incorrect one, before the incorrect one can be removed in a subsequent major release. |
|
@DaveCTurner do you mean to deprecate it for |
|
@olcbean 2 PRs would be normal, do the deprecation first, then removal in master as a followup. |
|
Yes, 2 PRs please, both against Anticipating future feedback (because I forgot last time I did something similar): there should be a test that the deprecation warning is emitted on the incorrect spelling. |
levenstein with levensHteinlevenstein in favor of levensHtein
|
Thanks @DaveCTurner |
| return new LevensteinDistance(); | ||
| } else if ("levenshtein".equals(distanceVal)) { | ||
| return new LevensteinDistance(); | ||
| // TODO Jaro and Winkler are 2 people - so apply same naming logic |
There was a problem hiding this comment.
It seems a shame to leave this TODO here. Would you care to fix that too?
|
Looking good. I added a request for some scope creep :) @elasticmachine please test this. |
| } | ||
|
|
||
| public void testLevensteinDeprecation() { | ||
| assertThat(DirectCandidateGeneratorBuilder.resolveDistance("levenstein"), equalTo(new LevensteinDistance())); |
There was a problem hiding this comment.
I might be missing something but I do not see a test that resolveDistance("levenshtein") does the right thing and does not produce a deprecation warning?
|
@DaveCTurner I will clean the Jaro and Winkler TODO once this PR is merged (I want to reuse the test I introduced in this PR) |
|
test this please |
* master: (41 commits) [Test] Fix AggregationsTests#testFromXContentWithRandomFields [DOC] Fix mathematical representation on interval (range) (elastic#27450) Update version check for CCS optional remote clusters Bump BWC version to 6.1.0 for elastic#27469 Adapt rest test BWC version after backport Fix dynamic mapping update generation. (elastic#27467) Use the primary_term field to identify parent documents (elastic#27469) Move composite aggregation to core (elastic#27474) Fix test BWC version after backport Protect shard splitting from illegal target shards (elastic#27468) Cross Cluster Search: make remote clusters optional (elastic#27182) [Docs] Fix broken bulleted lists (elastic#27470) Move resync request serialization assertion Fix resync request serialization Fix issue where pages aren't released (elastic#27459) Add YAML REST tests for filters bucket agg (elastic#27128) Remove tcp profile from low level nio channel (elastic#27441) [TEST] Fix `GeoShapeQueryTests#testPointsOnly` failure Transition transport apis to use void listeners (elastic#27440) AwaitsFix GeoShapeQueryTests#testPointsOnly elastic#27454 ...
|
retest this please |
|
@DaveCTurner If this looks good to you, would you mind pulling this in? I'm happy to walk you through the process, I think this would be your first time pulling a community PR in? |
Support both spellings thoughout 6.x, reporting the incorrect one as deprecated.
* es/master: (38 commits) Backport wait_for_initialiazing_shards to cluster health API Carry over version map size to prevent excessive resizing (#27516) Fix scroll query with a sort that is a prefix of the index sort (#27498) Delete shard store files before restoring a snapshot (#27476) Replace `delimited_payload_filter` by `delimited_payload` (#26625) CURRENT should not be a -SNAPSHOT version if build.snapshot is false (#27512) Fix merging of _meta field (#27352) Remove unused method (#27508) unmuted test, this has been fixed by #27397 Consolidate version numbering semantics (#27397) Add wait_for_no_initializing_shards to cluster health API (#27489) [TEST] use routing partition size based on the max routing shards of the second split Adjust CombinedDeletionPolicy for multiple commits (#27456) Update composite-aggregation.asciidoc Deprecate `levenstein` in favor of `levenshtein` (#27409) Automatically prepare indices for splitting (#27451) Validate `op_type` for `_create` (#27483) Minor ShapeBuilder cleanup muted test Decouple nio constructs from the tcp transport (#27484) ...
* es/6.x: (30 commits) Add wait_for_no_initializing_shards to cluster health API (#27489) Carry over version map size to prevent excessive resizing (#27516) Fix scroll query with a sort that is a prefix of the index sort (#27498) Delete shard store files before restoring a snapshot (#27476) CURRENT should not be a -SNAPSHOT version if build.snapshot is false (#27512) Fix merging of _meta field (#27352) test: do not run percolator query builder bwc test against 5.x versions Remove unused method (#27508) Consolidate version numbering semantics (#27397) Adjust CombinedDeletionPolicy for multiple commits (#27456) Minor ShapeBuilder cleanup [GEO] Deprecate ShapeBuilders and decouple geojson parse logic Improve docs for split API in 6.1/6.x (#27504) test: use correct pre 6.0.0-alpha1 format Update composite-aggregation.asciidoc Deprecate `levenstein` in favor of `levenshtein` (#27409) Decouple nio constructs from the tcp transport (#27484) Bump version from 6.1 to 6.2 Fix whitespace in Security.java Tighten which classes can exit ...
Fixes #27325