Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

[release/2.2] Fix ReadAsync not returning when cancellation token fires during FlushAsync#31432

Closed
pakrym wants to merge 2 commits intodotnet:release/2.2from
pakrym:pakrym/port-31212
Closed

[release/2.2] Fix ReadAsync not returning when cancellation token fires during FlushAsync#31432
pakrym wants to merge 2 commits intodotnet:release/2.2from
pakrym:pakrym/port-31212

Conversation

@pakrym
Copy link

@pakrym pakrym commented Jul 27, 2018

Fixes #31201

@pakrym pakrym requested a review from ahsonkhan July 27, 2018 19:24
@ahsonkhan ahsonkhan requested a review from danmoseley July 27, 2018 19:28
@danmoseley danmoseley requested a review from davidfowl July 27, 2018 20:17

completionData = default;

Copy link
Member

Choose a reason for hiding this comment

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

nit: whitespace

@karelz karelz added this to the 2.2 milestone Aug 15, 2018
@karelz
Copy link
Member

karelz commented Aug 15, 2018

@danmosemsft the PR is against 2.2, yet it has approved-2.1.5 label. What am I missing?
Do we need to manually port fixes into 2.2?

@karelz karelz changed the title Port 'Fix ReadAsync not returning when cancellation token fires during FlushAsync' to 2.2 [release/2.2] Fix ReadAsync not returning when cancellation token fires during FlushAsync Aug 15, 2018
@danmoseley
Copy link
Member

@karelz good point I did not notice that. @pakrym this was approved for 2.1.5. Please create a PR against release/2.1 and then move this servicing-approved-2.1.5 label to that PR.

Submissions into release/2.1 do flow to release/2.2 however with the branch not open for a while if you need it there sooner it makes sense to merge this PR also.

@karelz
Copy link
Member

karelz commented Aug 15, 2018

however with the branch not open for a while if you need it there sooner it makes sense to merge this PR also

@pakrym I would discourage from merging it into 2.2 branch, unless it is really, really needed there ASAP (in which case, please provide business justification). The key reason is to keep things simple and to keep integration costs lower further on. Thanks!

@danmoseley
Copy link
Member

@pakrym can you please link the 2.1 PR? I don't see it.

@danmoseley
Copy link
Member

You can mark it servicing-approved-2.1.5 if it is ready fairly soon.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants