[DOCS] Rewrite get mapping API docs to use new format #44085
Conversation
jrodewig
added
>docs
|
Pinging @elastic/es-docs |
|
Pinging @elastic/es-search |
jrodewig
changed the title
There was a problem hiding this comment.
Hopefully I didn't jump in too early if you weren't yet looking for a review, this PR just caught my eye.
I left a few comments, and also had one overall thought. Since these pages serve as the main API reference, I think many users consult them to get a quick idea/ refresher of what an API is about, and to look for a snippet to copy-and-paste. For that reason I wonder if it would be best to keep the 'example' section near the top. This would help maintain support for the most common 'use-cases' of these docs -- in the new format a user needs to scroll past a detailed list query + request parameters, many of which are quite rare to use.
|
|
||
| The get mapping API allows to retrieve mapping definitions for an index or | ||
| index/type. | ||
| Returns field mapping information for an index. |
There was a problem hiding this comment.
I think it would be nice to continue to refer to 'mapping definitions' as opposed to 'field mapping information', since this API returns more than just field mappings (for example dynamic_templates).
There was a problem hiding this comment.
Great point! Changed with 0f50a5a.
| (boolean) Indicates whether the type name is included in the response. Defaults | ||
| to `false`. | ||
|
|
||
| NOTE: Before 7.0.0, the 'mappings' definition used to include a type name. |
There was a problem hiding this comment.
I think it's important for this note to remain near the top of the page. In 7.0 we removed types from all APIs, which was a breaking change to several core, long-standing APIs. Having the note at the top of the page allows users to quickly debug the situation once they run into the change -- I'm not sure most users will benefit from the note when it's in this subsection.
There was a problem hiding this comment.
Agree. Fixed with 0d55000.
| [[indices-get-mapping-query-params]] | ||
| === {api-query-parms-title} | ||
|
|
||
| `allow_no_indices` (Optional):: |
There was a problem hiding this comment.
Should we link to the documentation on multiple indices (https://www.elastic.co/guide/en/elasticsearch/reference/current/multi-index.html), instead of repeating this information in every API reference?
There was a problem hiding this comment.
I think there is some value in listing the parameters here rather than directing people to a separate page.
As a compromise, I moved these parameters to a separate file and included them with
f676309. That way we can edit and reuse them from a single source but still make the content available everywhere.
Let me know if you think that works. I can also add an indicator that this is a rarely used or expert parameter if wanted.
There was a problem hiding this comment.
That seems like a good compromise to me, it will make the information easier to keep in sync.
| NOTE: Before 7.0.0, the 'mappings' definition used to include a type name. Although mappings | ||
| in responses no longer contain a type name by default, you can still request the old format | ||
| through the parameter include_type_name. For more details, please see <<removal-of-types>>. | ||
| `GET /<index>/_mapping` |
There was a problem hiding this comment.
In other documentation and technical communication, I've seen the style GET /{index}/_mapping. I haven't really seen us use angle brackets (a quick code search through *.asciidoc files confirms this trend).
There was a problem hiding this comment.
Done with 4e01d52. This will also match the current rest spec.
|
Thanks so much for your review @jjtibshirani. This is a beta/pilot of a new API reference template the writers are working on. I'll incorporate some changes there based on your feedback. I've moved the current example higher on the page to better enable grab-and-go for users just looking for a quick example. Let me know if you feel other examples (multiple/all indices) would also be helpful here. |
There was a problem hiding this comment.
Thanks for making all those changes!
Related to the placement of 'examples' -- to me it'd be more natural if the description came before the example, so it's not coming with no context. In fact, I'm wondering why there are two separate areas for a short description? Our current API documentation very often begins with a paragraph description that incorporates a simple example in-line. I think this reads nicely and puts the most helpful information up-front.
|
@jtibshirani Thank you again. I agree that the two descriptions now seem a bit much. I'll plan to talk this over with some of the other writers working on theAPI reference template and see if we can better incorporate those sections. In the meantime, I'm marking this and any PRs related to #43765 as a WIP. I won't merge these PRs until I've had a chance to let others review the template. |
|
@jrodewig thanks, marking them as WIP helps clarify the intention that this is a 'test drive' of the new template. |
|
Gonna close this for now. May re-open at a later date. |
With elastic/docs#938, we created a standard template for Elastic API Reference documentation.
This PR rewrites the get mapping API to use that template.
Relates to elastic/docs#937
Before
Get mapping - Before
After
Get mapping - After