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

Missing Implementation for Doc Examples #938

Merged
merged 2 commits into from
Aug 21, 2019

Conversation

philkra
Copy link
Contributor

@philkra philkra commented Aug 21, 2019

The Get API page had a missing implementation for the put mapping code snippet.

@philkra philkra requested a review from ezimuel August 21, 2019 06:49
@philkra philkra self-assigned this Aug 21, 2019
Copy link
Contributor

@ezimuel ezimuel left a comment

Choose a reason for hiding this comment

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

The only concern that I have is about the naming of the class Elasticsearch\Examples\Docs\Docs\Get. Why this is Get if there are examples with Put, Head, etc? Apart from that, LGTM.

@philkra
Copy link
Contributor Author

philkra commented Aug 21, 2019

Yes, it bothers me as well! Let me rework this to append a Page to every class name, e.g. GetPage. Or do you have a better suggestion for semantically reasonable naming suffix?

@ezimuel
Copy link
Contributor

ezimuel commented Aug 21, 2019

Proposal: use the same naming convention that we have for the endpoints. We will have small classes (unit tests) for each endpoint and this will also facilitate the search for examples. @philkra what do you think?

@ezimuel
Copy link
Contributor

ezimuel commented Aug 21, 2019

We discussed in a meeting with @philkra and it's ok for me to follow the actual naming convention for the doc examples. LGTM!

@philkra philkra merged commit 8692b46 into elastic:master Aug 21, 2019
@philkra philkra deleted the add-missing-doc-example branch August 21, 2019 09:45
@philkra philkra restored the add-missing-doc-example branch August 21, 2019 12:31
@philkra philkra deleted the add-missing-doc-example branch August 22, 2019 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants