Skip to content

[ML] Adjust wire serialization code after backport to 7.x #63062

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

Merged
merged 2 commits into from
Sep 30, 2020

Conversation

przemekwitek
Copy link
Contributor

This PR adjusts wire serialization code for Classification and AbstractAucRoc classes after backport to 7.x

Relates #62160

@elasticmachine
Copy link
Collaborator

Pinging @elastic/ml-core (:ml)

Copy link
Contributor

@droberts195 droberts195 left a comment

Choose a reason for hiding this comment

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

Since master only has to be wire compatible with 7.last you might as well remove the checks completely. It will save doing another PR for this in the future.

The only reason to keep BWC checks in master is if the code that's different in 7.x is so different that to discover it when doing a backport would be a major shock. But that's not the case here - the differences in the older versions are trivial.

@przemekwitek
Copy link
Contributor Author

Since master only has to be wire compatible with 7.last you might as well remove the checks completely. It will save doing another PR for this in the future.

Thanks for reminding me. Done.

Copy link
Contributor

@droberts195 droberts195 left a comment

Choose a reason for hiding this comment

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

LGTM

@przemekwitek przemekwitek merged commit 564da85 into elastic:master Sep 30, 2020
@przemekwitek przemekwitek deleted the auc_roc_version branch September 30, 2020 12:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants