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

Merge LongTermsAggregator and DoubleTermsAggregator. #7279

Closed

Conversation

Projects
None yet
3 participants
@jpountz
Copy link
Contributor

commented Aug 14, 2014

These two aggregators basically do exactly the same thing, they just interpret
bytes differently. This refactoring found an (unreleased) bug in the long terms
aggregator which didn't work correctly with duplicate values.

@jpountz jpountz added review labels Aug 14, 2014

@martijnvg

This comment has been minimized.

Copy link
Member

commented Aug 19, 2014

I like the removal of duplicated logic! LGTM

@jpountz jpountz removed the review label Aug 19, 2014

Aggregations: Merge LongTermsAggregator and DoubleTermsAggregator.
These two aggregators basically do exactly the same thing, they just interpret
bytes differently. This refactoring found an (unreleased) bug in the long terms
aggregator which didn't work correctly with duplicate values.

Close #7279

@jpountz jpountz added the enhancement label Aug 19, 2014

jpountz added a commit that referenced this pull request Aug 19, 2014

Aggregations: Merge LongTermsAggregator and DoubleTermsAggregator.
These two aggregators basically do exactly the same thing, they just interpret
bytes differently. This refactoring found an (unreleased) bug in the long terms
aggregator which didn't work correctly with duplicate values.

Close #7279

@jpountz jpountz closed this in 0f63e0a Aug 19, 2014

jpountz added a commit that referenced this pull request Sep 8, 2014

Aggregations: Merge LongTermsAggregator and DoubleTermsAggregator.
These two aggregators basically do exactly the same thing, they just interpret
bytes differently. This refactoring found an (unreleased) bug in the long terms
aggregator which didn't work correctly with duplicate values.

Close #7279

@clintongormley clintongormley changed the title Aggregations: Merge LongTermsAggregator and DoubleTermsAggregator. Merge LongTermsAggregator and DoubleTermsAggregator. Jun 7, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.