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.

Sign up for GitHub

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

Jump to bottom

[data.search.aggs]: Clean up TimeBuckets implementation #61996

Closed
wants to merge 7 commits into from
Closed

[data.search.aggs]: Clean up TimeBuckets implementation #61996

wants to merge 7 commits into from

Conversation

alexwizp
Copy link
Contributor

Part of #60130

Summary

Remove duplicated code from the Watch plugin related to TimeBuckets

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-arch (Team:AppArch)

@alexwizp alexwizp requested review from aalan3 and VladLasitsa and removed request for aalan3 March 31, 2020 15:22
@alexwizp alexwizp self-assigned this Mar 31, 2020
@alexwizp alexwizp requested review from lizozom and lukeelmers March 31, 2020 17:14
@alexwizp alexwizp added release_note:skip Skip the PR/issue when compiling release notes v7.8.0 v8.0.0 review Feature:NP Migration labels Mar 31, 2020
@alexwizp alexwizp marked this pull request as ready for review March 31, 2020 17:15
@alexwizp alexwizp requested a review from a team as a code owner March 31, 2020 17:15
lukeelmers
lukeelmers requested changes Mar 31, 2020
View reviewed changes
Copy link
Member

@lukeelmers lukeelmers left a comment

Choose a reason for hiding this comment

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

Summarizing our offline discussion: Let's wait to merge this and consider it as a possible last step once the other items in #60130 have been completed.

That way it is easier for us to make breaking changes during the refactoring as TimeBuckets remains internal to the data plugin. Once it is in a state we are happy with, we can revisit exposing to other plugins as a static export.

@VladLasitsa
Copy link
Contributor

@elasticmachine merge upstream

@elasticmachine elasticmachine requested a review from a team as a code owner April 2, 2020 15:23
sebelga
sebelga approved these changes Apr 3, 2020
View reviewed changes
Copy link
Contributor

@sebelga sebelga left a comment

Choose a reason for hiding this comment

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

Code owner change in watcher LGTM. 👍

@alexwizp
Merge remote-tracking branch 'upstream/master' into 60130
577ad00 
# Conflicts:
#	src/plugins/data/public/public.api.md
#	x-pack/plugins/watcher/public/plugin.ts
@alexwizp
Copy link
Contributor Author

@elasticmachine merge upstream

@alexwizp
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💔 Build Failed

Failed CI Steps

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@alexwizp alexwizp closed this Apr 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lukeelmers lukeelmers lukeelmers requested changes

@sebelga sebelga sebelga approved these changes

@VladLasitsa VladLasitsa Awaiting requested review from VladLasitsa

@lizozom lizozom Awaiting requested review from lizozom

Assignees

@alexwizp alexwizp

Labels
Feature:NP Migration release_note:skip Skip the PR/issue when compiling release notes review v7.8.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@alexwizp @elasticmachine @VladLasitsa @kibanamachine @lukeelmers @sebelga