document the new analysis-phonenumber plugin
#8469
Conversation
rursprung
requested review from
kolchfa-aws,
Naarcha-AWS,
vagimeli,
AMoo-Miki,
natebower,
dlvenable,
stephen-crawford and
epugh
as code owners
|
Thank you for submitting your PR. The PR states are In progress (or Draft) -> Tech review -> Doc review -> Editorial review -> Merged. Before you submit your PR for doc review, make sure the content is technically accurate. If you need help finding a tech reviewer, tag a maintainer. When you're ready for doc review, tag the assignee of this PR. The doc reviewer may push edits to the PR directly or leave comments and editorial suggestions for you to address (let us know in a comment if you have a preference). The doc reviewer will arrange for an editorial review. |
|
CC @reta I admittedly haven't rendered this locally yet to have a look at the final result. i hope the links & rendering are ok! |
_install-and-configure/additional-plugins/analysis-phonenumber-plugin.md
Outdated
Show resolved
Hide resolved
|
@kolchfa-aws: i guess this is ready (except that i haven't built it locally to check the links; but i'll be OOO for the next couple of days, so i think it'd be best to get this moving even if i'm not around) |
kolchfa-aws
added
v2.18.0
release-notes
|
Thank you, @rursprung! I think, for discoverability, it is best to put this information in the Text analysis section. Let me try to reorganize and push into this PR. |
|
@rursprung Doc review complete. Could you take a look and verify that my changes are accurate? It would be nice to provide a complete example (add a call to the |
this is part of opensearch-project/OpenSearch#11326. the actual implementation was done opensearch-project/OpenSearch#15915. see the commit message on the PR for further details. resolves opensearch-project#8389 Co-authored-by: Fanit Kolchina <[email protected]> Signed-off-by: Fanit Kolchina <[email protected]> Signed-off-by: Ralph Ursprung <[email protected]>
rursprung
force-pushed
the
document-analysis-phone-plugin
branch
from
1333073 to
d81da26
Compare
|
thanks a lot for your contribution @kolchfa-aws! i've added some examples, i hope they're ok in this form? if you'd like you can test them yourself on a recent checkout of OpenSearch by running |
Signed-off-by: Fanit Kolchina <[email protected]>
There was a problem hiding this comment.
@kolchfa-aws @rursprung Please see my comments and changes and let me know if you have any questions. Thanks!
Co-authored-by: Nathan Bower <[email protected]> Signed-off-by: kolchfa-aws <[email protected]>
Signed-off-by: kolchfa-aws <[email protected]>
Co-authored-by: Nathan Bower <[email protected]> Signed-off-by: kolchfa-aws <[email protected]>
|
@rursprung Editorial review complete. If you'd like to look over the changes, please do; otherwise, we're ready to merge. |
|
|
||
| ## Specifying a default region | ||
|
|
||
| You can optionally specify a default region for parsing phone numbers by providing the `phone-region` parameter within the analyzer. Valid phone regions are represented by ISO 3166 country codes. For more information, see [List of ISO 3166 country codes](https://en.wikipedia.org/wiki/List_of_ISO_3166_country_codes). |
There was a problem hiding this comment.
i'm not a native speaker, but i'd have expected "[..] the list [..]" here? (it probably was me who wrote it like this in the first place? 🫣)
| You can optionally specify a default region for parsing phone numbers by providing the `phone-region` parameter within the analyzer. Valid phone regions are represented by ISO 3166 country codes. For more information, see [List of ISO 3166 country codes](https://en.wikipedia.org/wiki/List_of_ISO_3166_country_codes). | |
| You can optionally specify a default region for parsing phone numbers by providing the `phone-region` parameter within the analyzer. Valid phone regions are represented by ISO 3166 country codes. For more information, see the [list of ISO 3166 country codes](https://en.wikipedia.org/wiki/List_of_ISO_3166_country_codes). |
There was a problem hiding this comment.
@rursprung Either way is correct 😄
| }, | ||
| "mappings": { | ||
| "properties": { | ||
| "phoneNumber": { |
There was a problem hiding this comment.
btw: do examples usually use camelCase, snake_case or kebab-case? this one currently uses kebab-case for the analyzer names (the actual phone-search analyzer does so too) and index name but camelCase for the field here (sorry, don't know how i ended up with this mix in this example)
There was a problem hiding this comment.
In general, OpenSearch uses snake_case in requests.
There was a problem hiding this comment.
so this should be phone_number? or is it not worth the effort to discuss this for the documentation? 😅
There was a problem hiding this comment.
Ideally, it should all be snake case, so yes, phone_number is preferred. That said, even our own alerting plugin uses camel case. I think it would be nice to change to snake case 😄
| ``` | ||
| {% include copy-curl.html %} | ||
|
|
||
| Is parsed into the following tokens: |
There was a problem hiding this comment.
i had is lowercase as it's the continuation of line 113. now it feels like a new sentence but isn't one (lacking a subject)
There was a problem hiding this comment.
@rursprung I'm not a huge fan of the split-sentence structure, generally, and would prefer that we not use it because it results in this type of issue (case in point: I'd prefer that new lines/sentences/phrases not begin with lowercase letters).
Signed-off-by: kolchfa-aws <[email protected]>
|
@rursprung @natebower I changed to |
Description
this is part of opensearch-project/OpenSearch#11326. the actual implementation was done opensearch-project/OpenSearch#15915. see the commit message on the PR for further details.
Issues Resolved
resolves #8389
Version
this has been merged into 2.18.0 (opensearch-project/OpenSearch#16187).
Frontend features
n/a
Checklist
For more information on following Developer Certificate of Origin and signing off your commits, please check here.