-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Update date_histogram docs #56922
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
Update date_histogram docs #56922
Conversation
* Make it more clear that you can use `month` or `1M`. * Explain rounding rules * Consistently use "time zone" instead of "timezone". It looks like both are right but I see "time zone" much more. And the parameter in elasticsearch is `time_zone` so we may as well line up. Closes elastic#56760
|
Pinging @elastic/es-analytics-geo (:Analytics/Aggregations) |
|
Pinging @elastic/es-docs (>docs) |
|
Would anyone from @elastic/es-docs like to review this one? |
jrodewig
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @nik9000. This looks good for the most part.
There are a few typos and misspellings that need to be fixed.
Otherwise, just some non-blocking rewording suggestions or
questions/thoughts.
docs/reference/aggregations/bucket/datehistogram-aggregation.asciidoc
Outdated
Show resolved
Hide resolved
docs/reference/aggregations/bucket/datehistogram-aggregation.asciidoc
Outdated
Show resolved
Hide resolved
docs/reference/aggregations/bucket/datehistogram-aggregation.asciidoc
Outdated
Show resolved
Hide resolved
docs/reference/aggregations/bucket/datehistogram-aggregation.asciidoc
Outdated
Show resolved
Hide resolved
docs/reference/aggregations/bucket/datehistogram-aggregation.asciidoc
Outdated
Show resolved
Hide resolved
| The accepted units for fixed intervals are: | ||
|
|
||
| milliseconds (ms) :: | ||
| milliseconds (`ms`) :: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having an empty def feels weird to me. Maybe move the expanded word within the def?
`ms`::
Milliseconds
Would need to do this for all the definitions tho.
docs/reference/aggregations/bucket/datehistogram-aggregation.asciidoc
Outdated
Show resolved
Hide resolved
docs/reference/aggregations/bucket/datehistogram-aggregation.asciidoc
Outdated
Show resolved
Hide resolved
docs/reference/aggregations/bucket/datehistogram-aggregation.asciidoc
Outdated
Show resolved
Hide resolved
docs/reference/aggregations/bucket/datehistogram-aggregation.asciidoc
Outdated
Show resolved
Hide resolved
…sciidoc Co-authored-by: James Rodewig <[email protected]>
…sciidoc Co-authored-by: James Rodewig <[email protected]>
…sciidoc Co-authored-by: James Rodewig <[email protected]>
Co-authored-by: James Rodewig <[email protected]>
|
Thanks @jrodewig ! I committed your suggestions and added a few small things. I'll recheck the preview once it builds and merge. |
* Make it more clear that you can use `month` or `1M`. * Explain rounding rules * Consistently use "time zone" instead of "timezone". It looks like both are right but I see "time zone" much more. And the parameter in elasticsearch is `time_zone` so we may as well line up. Closes elastic#56760 Co-authored-by: James Rodewig <[email protected]>
* Make it more clear that you can use `month` or `1M`. * Explain rounding rules * Consistently use "time zone" instead of "timezone". It looks like both are right but I see "time zone" much more. And the parameter in elasticsearch is `time_zone` so we may as well line up. Closes #56760 Co-authored-by: James Rodewig <[email protected]>
monthor1M.are right but I see "time zone" much more. And the parameter in
elasticsearch is
time_zoneso we may as well line up.Closes #56760