Skip to content

Conversation

@benwtrent
Copy link
Member

Simple change, it should have been strictly parsed from the get go....

@elasticmachine
Copy link
Collaborator

Pinging @elastic/ml-core

Copy link
Contributor

@dimitris-athanasiou dimitris-athanasiou left a comment

Choose a reason for hiding this comment

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

LGTM

@hendrikmuhs
Copy link

may I play devils advocate?

If we missed this in the 1st place it means we lack a test.

@benwtrent
Copy link
Member Author

@elasticmachine
run elasticsearch-ci/1
run elasticsearch-ci/2
run elasticsearch-ci/bwc
run elasticsearch-ci/default-distro

@benwtrent
Copy link
Member Author

run elasticsearch-ci/bwc

Copy link

@hendrikmuhs hendrikmuhs left a comment

Choose a reason for hiding this comment

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

LGTM

@benwtrent
Copy link
Member Author

run elasticsearch-ci/bwc

2 similar comments
@benwtrent
Copy link
Member Author

run elasticsearch-ci/bwc

@benwtrent
Copy link
Member Author

run elasticsearch-ci/bwc

@benwtrent
Copy link
Member Author

run elasticsearch-ci/bwc

@benwtrent benwtrent merged commit 8f5e32a into elastic:master Mar 9, 2019
@benwtrent benwtrent deleted the feature/data-frame-make-preview-strictly-parse-config branch March 9, 2019 04:11
benwtrent added a commit to benwtrent/elasticsearch that referenced this pull request Mar 9, 2019
…39713)

* [Data-Frame] make the config be strictly parsed on _preview

* adding test to verify strictly parsing

* adjusting test after master merge
benwtrent added a commit that referenced this pull request Mar 9, 2019
…39873)

* [Data-Frame] make the config be strictly parsed on _preview

* adding test to verify strictly parsing

* adjusting test after master merge
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.

5 participants