-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Updated the keys in the percentile aggregation tests to be strings #27355
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
Conversation
9c09741 to
8ee27da
Compare
cbuescher
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.
LGTM
|
Pinging @elastic/es-search-aggs |
|
@karmi are you picking this one up? |
|
@bleskes , argh, went totally under my radar, I'll look into it... |
|
@karmi could you take a look at the current version on master that is in conflict with this PR please? It changed the match clauses so I'm not sure anymore if your PR still makes sense or can be closed (don't know how the ruby test runner works with the current style) |
|
I will, I’m on vacation since last week, so when I get back to the computer! :)
…--
Sent on the go
On 8 May 2018, at 13:49, Christoph Büscher ***@***.***> wrote:
@karmi could you take a look at the current version on master that is in conflict with this PR please? It changed the match clauses so I'm not sure anymore if your PR still makes sense or can be closed (don't know how the ruby test runner works with the current style)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
I'm going to close this for now indeed — I'll create a fresh PR from master if I'd run into the issue again. Thanks for the ping @bleskes and @cbuescher ! |
Following in the path of #26905, this patch updates another set of keys
to be strings instead of numbers, otherwise the Ruby runner fails.