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

[docs] Update with the latest changes and use compilable examples #52

Merged
merged 5 commits into from
Dec 7, 2021

Conversation

swallez
Copy link
Member

@swallez swallez commented Dec 6, 2021

Update/cleanup documentation and refactor code examples as included sections of test code, so that code snippets in the docs are compiled and tested.

@swallez swallez added Area: Documentation Improvements or additions to documentation v8.0.0 v7.16.0 labels Dec 6, 2021
@swallez swallez self-assigned this Dec 6, 2021
@swallez swallez requested a review from szabosteve December 6, 2021 22:22
swallez and others added 2 commits December 7, 2021 11:29
Co-authored-by: István Zoltán Szabó <istvan.szabo@elastic.co>
Co-authored-by: István Zoltán Szabó <istvan.szabo@elastic.co>
@swallez
Copy link
Member Author

swallez commented Dec 7, 2021

@szabosteve as discussed, I've added the test code in a separate commit in both the main and 7.16 branches and removed it from this PR.

and ease distinguishing what identifiers belong to the API or to the framework.
Client is built, such as `Query._kind()`. These methods and properties are
prefixed with an underscore to both avoid any naming conflicts with API names,
and allow to easily distinguish what belongs to the API from what belongs to the
Copy link
Contributor

@technige technige Dec 7, 2021

Choose a reason for hiding this comment

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

"and as an easy way to distinguish the API from the framework"

It becomes particularly useful with complex nested queries like the one below,
taken from the
Note in the above example that builder variables are only used to start a chain
of property setters. Their name therefore doesn't matter much and can be shortened
Copy link
Contributor

Choose a reason for hiding this comment

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

"The names of these variables are therefore unimportant and can be shortened"

Java syntax, we name them with an underscore followed by the depth of the item,
i.e. `_0`, `_1`, and so on. This removes the need for finding names and makes
the code easier to read by reducing the number of identifiers. Correct indentation
also allows the structure of the query to stand out.
Copy link
Contributor

Choose a reason for hiding this comment

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

"This example also highlights a useful and recommended naming convention for builder parameters in deeply nested structures. For lambda arguments, Kotlin provides it and Scala allows use of _; this can be approximated in Java by using an underscore prefix followed by a number representing the depth level (i.e. _0, _1, and so on). Not only does this remove the need to generate throw-away variable names, but it also improves code readability."


Properties of type `List` and `Map` are exposed by object builders as a set of overloaded
methods that _update_ the property value, by appending to lists and adding new entries to maps
(or replacing existing ones).
Copy link
Contributor

Choose a reason for hiding this comment

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

It it worth clarifying that these are "additive-only" and should not be considered regular collection objects, due to a lack of "remove" methods, and similar?

Copy link
Contributor

@technige technige left a comment

Choose a reason for hiding this comment

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

Some minor suggestions, but all looks good in general :)

@swallez
Copy link
Member Author

swallez commented Dec 7, 2021

@technige I added a commit with your suggestions and also added a paragraph explaning that lists and maps are never null even if undefined in the ES response.

@technige technige self-requested a review December 7, 2021 14:19
Copy link
Contributor

@technige technige left a comment

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

@szabosteve szabosteve left a comment

Choose a reason for hiding this comment

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

LGTM2

@swallez swallez merged commit e8423e1 into main Dec 7, 2021
@swallez swallez deleted the update-docs branch December 7, 2021 14:40
swallez added a commit that referenced this pull request Dec 7, 2021
Co-authored-by: István Zoltán Szabó <istvan.szabo@elastic.co>
swallez added a commit that referenced this pull request Dec 7, 2021
… (#53)

Co-authored-by: István Zoltán Szabó <istvan.szabo@elastic.co>
swallez added a commit that referenced this pull request Dec 22, 2021
Co-authored-by: István Zoltán Szabó <istvan.szabo@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Documentation Improvements or additions to documentation v7.16.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants