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

[DOCS] Edits 6.5.4 release notes entries #346

Merged
merged 2 commits into from
Dec 18, 2018
Merged

[DOCS] Edits 6.5.4 release notes entries #346

merged 2 commits into from
Dec 18, 2018

Conversation

lcawl
Copy link
Contributor

@lcawl lcawl commented Dec 17, 2018

This PR edits the ml-cpp entries for inclusion in the Elasticsearch 6.5.4 release notes. In particular, it references the following PR: #327

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.

I left one comment but feel free to merge once you've addressed it as you think best.

in apparently shifted model plot with respect to the data and increased errors in forecasts.
Corrects query times for model bounds and forecasts in the bucket to match the
times assigned to the samples that are added to the model for each bucket. For
long bucket lengths, this fix could result in apparently shifted model bounds
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this line needs changing as it refers to the situation before the fix. One possibility would be to simply remove the word "fix". Or if you want the word "fix" to appear somewhere, another way would be to completely flip the paragraph:

Fixes a problem that could result in shifted model bounds and increased forecast errors with long bucket spans. The change was to correct query times for model bounds and forecasts in the bucket to match the times assigned to the samples that are added to the model for each bucket. (See {ml-pull}327[#327].)

Copy link
Contributor

Choose a reason for hiding this comment

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

or possibly "fix" -> "bug", but I think I maybe prefer Dave's suggestion

@tveasey
Copy link
Contributor

tveasey commented Dec 18, 2018

LGTM2 (with the issue resolved).

@lcawl
Copy link
Contributor Author

lcawl commented Dec 18, 2018

Thanks for the feedback! I've updated the text

@lcawl lcawl merged commit e529df0 into elastic:6.5 Dec 18, 2018
@lcawl lcawl deleted the ml-654 branch December 18, 2018 18:36
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.

3 participants