Consistent encoder names #29492

russcam · 2018-04-12T03:43:41Z

This commit updates encoder names to be consistent within documentation
and align with snake casing convention.

cbuescher

@russcam I agree that it would be better if the encoder names consistently use one of the variants. I don't know enough to say if snake_case is the current "offical" variant since neither of the existing variant seems deprecated, but I added a few comments to change some other occurances in tests and code if we go with this version. Do you think we should we also start deprecating the other options? Has there been an issue where this was discussed already?

cbuescher · 2018-04-12T13:57:37Z

docs/plugins/analysis-phonetic.asciidoc

    `caverphone2`, `cologne`, `nysiis`, `koelnerphonetik`, `haasephonetik`,
-    `beidermorse`, `daitch_mokotoff`.
+    `beider_morse`, `daitch_mokotoff`.


If this is changes, beidermorse in L28 should be changed as well. Also there is an occurence in plugins/analysis-phonetic/src/test/resources/org/elasticsearch/index/analysis/phonetic-1.yml and plugins/analysis-phonetic/src/test/java/org/elasticsearch/index/analysis/AnalysisPhoneticFactoryTests.java where we should then also test with this name, although the other variant is supported as well.

cbuescher · 2018-04-12T13:58:20Z

docs/plugins/analysis-phonetic.asciidoc

@@ -17,9 +17,9 @@ The `phonetic` token filter takes the following settings:
 `encoder`::

    Which phonetic encoder to use.  Accepts `metaphone` (default),
-    `doublemetaphone`, `soundex`, `refinedsoundex`, `caverphone1`,
+    `double_metaphone`, `soundex`, `refined_soundex`, `caverphone1`,


Maybe also change "refinedsoundec" in plugins/analysis-phonetic/src/test/resources/org/elasticsearch/index/analysis/phonetic-1.yml then

cbuescher · 2018-04-12T13:59:18Z

docs/plugins/analysis-phonetic.asciidoc

@@ -17,9 +17,9 @@ The `phonetic` token filter takes the following settings:
 `encoder`::

    Which phonetic encoder to use.  Accepts `metaphone` (default),
-    `doublemetaphone`, `soundex`, `refinedsoundex`, `caverphone1`,
+    `double_metaphone`, `soundex`, `refined_soundex`, `caverphone1`,


Mabe also change doublemetaphone in plugins/analysis-phonetic/src/test/java/org/elasticsearch/index/analysis/AnalysisPhoneticFactoryTests.java and plugins/analysis-phonetic/src/test/resources/org/elasticsearch/index/analysis/phonetic-1.yml then

elasticmachine · 2018-04-13T08:50:34Z

Pinging @elastic/es-search-aggs

cbuescher · 2018-05-30T10:32:44Z

@russcam just checking in, are you still interested in making this change? If not, I'd like to close this PR.

russcam · 2018-06-04T04:46:46Z

I'd still like to make this change @cbuescher. As far as I have seen whilst working on the .NET client, the preference seems to be towards snake-casing, but perhaps @clintongormley has the authoritative answer?

Irrespective of what the correct name should be, I'd still like to make the change, to make the documentation consistent with itself e.g. if beider_morse is used in one place in docs, it should be used in all in docs.

clintongormley · 2018-06-08T19:29:58Z

My personal preference would be snakecase

cbuescher · 2018-07-18T10:37:02Z

@russcam any chance to get this one ready to merge?

russcam · 2018-07-18T11:43:33Z

thanks for the ping @cbuescher, I'll take a look first thing tomorrow 👍

This commit updates tests and docs to use snake_case encoder names for consistency. Addresses comments on #29492

russcam · 2018-07-20T02:15:12Z

I've addressed the comments in relation to updating names in phonetic-1.yml and AnalysisPhoneticFactoryTests.java to use snake_casing. @cbuescher would you mind taking another look when you get a chance?

cbuescher · 2018-07-20T10:34:18Z

...is-phonetic/src/test/java/org/elasticsearch/index/analysis/AnalysisPhoneticFactoryTests.java

-        filters.put("beidermorse", PhoneticTokenFilterFactory.class);
-        filters.put("doublemetaphone", PhoneticTokenFilterFactory.class);
+        filters.put("beider_morse", PhoneticTokenFilterFactory.class);
+        filters.put("double_metaphone", PhoneticTokenFilterFactory.class);


Sorry, I was wrong with my previous comment that this needs changing. This will actually fail the test now. I think the test is based on class name lookup, and the "_" doesn't work with this.

cbuescher

@russcam thanks for the update, I left a comment. Sorry about the test change, my mistake. Could you merge in current master or rebase once you change the test back so CI can run on a current version?

This commit updates encoder names to be consistent within documentation and align with snake casing convention.

This commit updates tests and docs to use snake_case encoder names for consistency. Addresses comments on #29492

This commit reverts the filter names used for testing back to what they were, as the name is used for class name lookup.

russcam · 2018-07-22T23:28:09Z

Rebased against latest master and reverted filter names in AnalysisPhoneticFactoryTests back

cbuescher

Thanks for the changes, LGTM

russcam · 2018-07-23T07:16:56Z

Thanks @cbuescher. Are you OK for me to merge and backport (back to 6.3)?

cbuescher · 2018-07-23T09:29:00Z

@russcam yes please. I'd merge this to 6.x only since it is not a really important bug-fix for the 6.3 line though.

This commit updates encoder names to be consistent within documentation and align with snake casing convention. (cherry picked from commit e2b665c)

russcam · 2018-07-23T23:31:24Z

ported to 6.x: 990afd0

* 6.x: Security: revert to old way of merging automata (#32254) Fix a test bug in RangeQueryBuilderTests introduced in the field aliases backport. Introduce Application Privileges with support for Kibana RBAC (#32309) Undo a debugging change that snuck in during the field aliases merge. [test] port linux package packaging tests (#31943) Painless: Update More Methods to New Naming Scheme (#32305) Tribe: Add error with secure settings copied to tribe (#32298) Add V_6_3_3 version constant Add ERR to ranking evaluation documentation (#32314) [DOCS] Added link to 6.3.2 RNs [DOCS] Updates 6.3.2 release notes with PRs from ml-cpp repo (#32334) [Kerberos] Add Kerberos authentication support (#32263) [ML] Extract persistent task methods from MlMetadata (#32319) Backport - Add Snapshots Status API to High Level Rest Client (#32295) Make release notes ignore the `>test-failure` label. (#31309) [DOCS] Adds release highlights for search for 6.4 (#32095) Allow Integ Tests to run in a FIPS-140 JVM (#32316) Add support for field aliases to 6.x. (#32184) Register ERR metric with NamedXContentRegistry (#32320) fixes broken build for third-party-tests (#32315) Relates #31918 / Closes infra/issues/6085 [DOCS] Rollup Caps API incorrectly mentions GET Jobs API (#32280) Rest HL client: Add put watch action (#32026) (#32191) Add WeightedAvg metric aggregation (#31037) Consistent encoder names (#29492) Switch monitoring to new style Requests (#32255) specify subdirs of lib, bin, modules in package (#32253) Rename ranking evaluation `quality_level` to `metric_score` (#32168) Add new permission for JDK11 to load JAAS libraries (#32132) Switch x-pack:core to new style Requests (#32252) Watcher: Store username on watch execution (#31873) Silence SSL reload test that fails on JDK 11 Painless: Clean up add methods in PainlessLookup (#32258) CCE when re-throwing "shard not available" exception in TransportShardMultiGetAction (#32185) Fail shard if IndexShard#storeStats runs into an IOException (#32241) Fix `range` queries on `_type` field for singe type indices (#31756) (#32161) AwaitsFix RecoveryIT#testHistoryUUIDIsGenerated Add new fields to monitoring template for Beats state (#32085) (#32273) [TEST] improve REST high-level client naming conventions check (#32244) Check that client methods match API defined in the REST spec (#31825)

* master: Security: revert to old way of merging automata (#32254) Networking: Fix test leaking buffer (#32296) Undo a debugging change that snuck in during the field aliases merge. Painless: Update More Methods to New Naming Scheme (#32305) [TEST] Fix assumeFalse -> assumeTrue in SSLReloadIntegTests Ingest: Support integer and long hex values in convert (#32213) Introduce fips_mode setting and associated checks (#32326) Add V_6_3_3 version constant [DOCS] Removed extraneous callout number. Rest HL client: Add put license action (#32214) Add ERR to ranking evaluation documentation (#32314) Introduce Application Privileges with support for Kibana RBAC (#32309) Build: Shadow x-pack:protocol into x-pack:plugin:core (#32240) [Kerberos] Add Kerberos authentication support (#32263) [ML] Extract persistent task methods from MlMetadata (#32319) Add Restore Snapshot High Level REST API Register ERR metric with NamedXContentRegistry (#32320) fixes broken build for third-party-tests (#32315) Allow Integ Tests to run in a FIPS-140 JVM (#31989) [DOCS] Rollup Caps API incorrectly mentions GET Jobs API (#32280) awaitsfix testRandomClusterStateUpdates [TEST] add version skip to weighted_avg tests Consistent encoder names (#29492) Add WeightedAvg metric aggregation (#31037) Switch monitoring to new style Requests (#32255) Rename ranking evaluation `quality_level` to `metric_score` (#32168) Fix a test bug around nested aggregations and field aliases. (#32287) Add new permission for JDK11 to load JAAS libraries (#32132) Silence SSL reload test that fails on JDK 11 [test] package pre-install java check (#32259) specify subdirs of lib, bin, modules in package (#32253) Switch x-pack:core to new style Requests (#32252) awaitsfix SSLConfigurationReloaderTests Painless: Clean up add methods in PainlessLookup (#32258) Fail shard if IndexShard#storeStats runs into an IOException (#32241) AwaitsFix RecoveryIT#testHistoryUUIDIsGenerated Remove unnecessary warning supressions (#32250) CCE when re-throwing "shard not available" exception in TransportShardMultiGetAction (#32185) Add new fields to monitoring template for Beats state (#32085)

cbuescher requested changes Apr 12, 2018

View reviewed changes

colings86 added the :Search Relevance/Analysis How text is split into tokens label Apr 13, 2018

cbuescher self-assigned this Apr 13, 2018

colings86 added the >enhancement label Apr 24, 2018

russcam added a commit that referenced this pull request Jul 20, 2018

Use consistent encoder names across docs and tests

9bf06aa

This commit updates tests and docs to use snake_case encoder names for consistency. Addresses comments on #29492

cbuescher reviewed Jul 20, 2018

View reviewed changes

cbuescher requested changes Jul 20, 2018

View reviewed changes

russcam and others added 3 commits July 23, 2018 09:27

Consistent encoder names

ae91d17

This commit updates encoder names to be consistent within documentation and align with snake casing convention.

Use consistent encoder names across docs and tests

904f7af

This commit updates tests and docs to use snake_case encoder names for consistency. Addresses comments on #29492

Revert test filter names back

e65632a

This commit reverts the filter names used for testing back to what they were, as the name is used for class name lookup.

russcam force-pushed the russcam-patch-6-1 branch from 7f0f8b9 to e65632a Compare July 22, 2018 23:27

cbuescher approved these changes Jul 23, 2018

View reviewed changes

cbuescher added v7.0.0 v6.4.0 labels Jul 23, 2018

russcam merged commit e2b665c into master Jul 23, 2018

russcam added a commit that referenced this pull request Jul 23, 2018

Consistent encoder names (#29492)

990afd0

This commit updates encoder names to be consistent within documentation and align with snake casing convention. (cherry picked from commit e2b665c)

russcam deleted the russcam-patch-6-1 branch July 23, 2018 23:31

Mpdreamz mentioned this pull request Sep 24, 2018

[meta] 6.4.0 release elastic/elasticsearch-net#3397

Closed

89 tasks

colings86 added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Consistent encoder names #29492

Consistent encoder names #29492

russcam commented Apr 12, 2018

cbuescher left a comment

cbuescher Apr 12, 2018 •

edited

Loading

cbuescher Apr 12, 2018

cbuescher Apr 12, 2018

elasticmachine commented Apr 13, 2018

cbuescher commented May 30, 2018

russcam commented Jun 4, 2018

clintongormley commented Jun 8, 2018

cbuescher commented Jul 18, 2018

russcam commented Jul 18, 2018

russcam commented Jul 20, 2018

cbuescher Jul 20, 2018

cbuescher left a comment

russcam commented Jul 22, 2018

cbuescher left a comment

russcam commented Jul 23, 2018

cbuescher commented Jul 23, 2018 •

edited

Loading

russcam commented Jul 23, 2018

Consistent encoder names #29492

Consistent encoder names #29492

Conversation

russcam commented Apr 12, 2018

cbuescher left a comment

Choose a reason for hiding this comment

cbuescher Apr 12, 2018 • edited Loading

Choose a reason for hiding this comment

cbuescher Apr 12, 2018

Choose a reason for hiding this comment

cbuescher Apr 12, 2018

Choose a reason for hiding this comment

elasticmachine commented Apr 13, 2018

cbuescher commented May 30, 2018

russcam commented Jun 4, 2018

clintongormley commented Jun 8, 2018

cbuescher commented Jul 18, 2018

russcam commented Jul 18, 2018

russcam commented Jul 20, 2018

cbuescher Jul 20, 2018

Choose a reason for hiding this comment

cbuescher left a comment

Choose a reason for hiding this comment

russcam commented Jul 22, 2018

cbuescher left a comment

Choose a reason for hiding this comment

russcam commented Jul 23, 2018

cbuescher commented Jul 23, 2018 • edited Loading

russcam commented Jul 23, 2018

cbuescher Apr 12, 2018 •

edited

Loading

cbuescher commented Jul 23, 2018 •

edited

Loading