Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove "include_type_name" from most places in docs #37556

Closed
wants to merge 4 commits into from

Conversation

cbuescher
Copy link
Member

The "include_type_name" parameter was temporarily introduced in #37285 to facilitate
moving the default parameter setting to "false" in many places in the documentation
code snippets. Most of the places can simply be reverted without causing errors now
which this change does. The remaining nine files with approx. 37 cases where removing
the parameter causes an error will be adressed in a follow up.

@cbuescher cbuescher requested a review from jtibshirani January 16, 2019 23:05
@cbuescher cbuescher added >docs General docs changes :Search Foundations/Mapping Index mappings, including merging and defining field types v7.0.0 >refactoring labels Jan 16, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search

@@ -11,7 +11,7 @@ For example, let's say we have an index of questions and answers. The answer typ

[source,js]
--------------------------------------------------
PUT child_example?include_type_name=true
PUT child_example
{
"mappings": {
"_doc": {
Copy link
Contributor

@jtibshirani jtibshirani Jan 16, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To update this example, we'll also need to remove the type name _doc from the mappings definition. In my original PR, I wasn't able to figure out a good way to do this programmatically.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, I was under the false impression that the parameter was added solely to prevent failing docs-tests.
In that case probably large parts of this PR are void, but I think at least a huge chunk of the cases e.g. under "analysis" don't touch mappings at all, so I'll see if I can skim these quickly and maybe just restrict the PR to those simple cases.

@jtibshirani
Copy link
Contributor

jtibshirani commented Jan 17, 2019

@cbuescher thanks a lot for looking into this. I actually don't think most uses of include_type_name=true can simply be reverted without the request now being invalid. In this PR, in several requests where include_type_name=true is removed, I still see uses of the _doc type name in the mappings definition.

@cbuescher
Copy link
Member Author

@jtibshirani sorry, I see know why this PR went a bit too far and there is actually quiet a bit of manual work. I took a step back and tried to at least eliminate the use of the "include_type_name" parameter where it seems quite clear the request doesn't touch mappings. I opened #37568 for that which leaves about 159 locations untouched, but still manages to remove about 140 instances. Let me know what you think. I will close this PR in favour of #37568

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>docs General docs changes >refactoring :Search Foundations/Mapping Index mappings, including merging and defining field types v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants