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

Can we remove abstract InternalMetricsAggregation? #23326

Merged

Conversation

cbuescher
Copy link
Member

This class doesn't seem to do much other than to group together
certain types of aggregations. If that is the reason for its existence, we can
probably use a marker interface for that.

This class doesn't seem to serve any purpose other than to group together
certain types of aggregations. If that is the reason for its existence, we can
probably use a marker interface for that.
@s1monw
Copy link
Contributor

s1monw commented Feb 23, 2017

since this issue matches remove AND Internal.*Aggregation I am +1 :)

Copy link
Contributor

@s1monw s1monw left a comment

Choose a reason for hiding this comment

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

LGTM

@cbuescher cbuescher merged commit 8b1b152 into elastic:master Feb 23, 2017
cbuescher added a commit that referenced this pull request Feb 23, 2017
This class doesn't seem to do much other than to group together
certain types of aggregations.
@cbuescher
Copy link
Member Author

On 5x. with fdc674a

jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Feb 24, 2017
* master: (54 commits)
  Keep the pipeline handler queue small initially
  Do not create String instances in 'Strings' methods accepting StringBuilder (elastic#22907)
  Tests: fix AwsS3ServiceImplTests
  Remove abstract InternalMetricsAggregation class (elastic#23326)
  Add BulkRequest support to High Level Rest client (elastic#23312)
  Wrap getCredentials() in a doPrivileged() block (elastic#23297)
  Respect promises on pipelined responses
  Align REST specs for HEAD requests
  Remove unnecessary result sorting in SearchPhaseController (elastic#23321)
  Fix SamplerAggregatorTests to have stable and predictable docIds
  Tests: Ensure multi node integ tests wait on first node
  Relocate a comment in HttpPipeliningHandler
  Add comments to HttpPipeliningHandler
  [TEST] Fix incorrect test cluster name in cluster health doc tests
  Build: Change location in zip of license and notice inclusion for plugins (elastic#23316)
  Script: Fix value of `ctx._now` to be current epoch time in milliseconds (elastic#23175)
  Build: Rework integ test setup and shutdown to ensure stop runs when desired (elastic#23304)
  Handle long overflow when adding paths' totals
  Don't set local node on cluster state used for node join validation (elastic#23311)
  Ensure that releasing listener is called
  ...
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.

None yet

3 participants