-
Notifications
You must be signed in to change notification settings - Fork 65
[DOCS] Updates changelog for 6.6.0 #366
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
docs/CHANGELOG.asciidoc
Outdated
@@ -40,10 +40,17 @@ | |||
|
|||
=== Bug Fixes | |||
|
|||
Fix cause of "Sample out of bounds" error message (See {ml-pull}355[355].} | |||
Fix cause of "Sample out of bounds" error message. {ml-pull}335[335] |
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.
Should there be a #
before the 335
in square brackets?
docs/CHANGELOG.asciidoc
Outdated
Fix cause of "Sample out of bounds" error message. {ml-pull}335[335] | ||
|
||
Fix hanging autodetect process after it receives a warning about failed | ||
cleanup of forecasts. {ml-pull}352[#352] (issue: {ml-issue}350[#350]) |
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.
Maybe something more like this?
Fix hang during job close after failure to clean up temporary storage used by forecasting.
Closing the job is where the end user suffers from the problem.
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.
New forecast requests are also not executed after this happened, plus it requires a large job to run into this, maybe:
Fix hang during job close and forecast execution after temporary storage couldn't be cleaned up when forecasting a large job.
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.
Thanks, I've drafted another version of that description.
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.
I suggest to move the entry to 6.6.0, it's listed for 6.5.5 but 6.5.5 has been cancelled (or postponed) and to me it seems it is unlikely that there will be a 6.5.5 before 6.6.0.
This wasn't foreseeable when I backported the fix.
@droberts195, @lcawl what do you think?
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.
The PR listed in the 6.5.5 section is the exact same one that is already listed on lines 45/46 in the 6.6.0 section, so it's probably easiest to just delete the 6.5.5 section.
Or you could take inspiration from the current 6.5.5 description to adjust the 6.6.0 description. They're both intending to say the same thing but feel free to adjust the one we keep if you think one way of saying it is clearer.
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.
didn't see that!
I agree, lets delete the 6.5.5 and take the nicer one for 6.6.0. I have no strong feeling which one.
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.
Thanks! I've removed the duplicate and will now copy this 6.6 information to the Elasticsearch Release Notes.
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
This PR adds the following PRs to the changelog for 6.6.0:
#338
#357