Skip to content

Remove Net5CompatFileStreamStrategy. - #57735

Merged
jeffhandley merged 2 commits into
dotnet:mainfrom
teo-tsirpanis:net5compatstrategy-remove
Oct 9, 2021
Merged

Remove Net5CompatFileStreamStrategy.#57735
jeffhandley merged 2 commits into
dotnet:mainfrom
teo-tsirpanis:net5compatstrategy-remove

Conversation

@teo-tsirpanis

@teo-tsirpanisteo-tsirpanis commented Aug 19, 2021

Copy link
Copy Markdown
Contributor

Fixes#55196. I leave it here to be merged when seen fit.

c.c. @adamsitnik

@ghostghost added area-System.IO community-contribution Indicates that the PR has been added by a community member labels Aug 19, 2021
@ghost

ghost commented Aug 19, 2021

Copy link
Copy Markdown

Tagging subscribers to this area: @dotnet/area-system-io
See info in area-owners.md if you want to be subscribed.

Issue Details

Fixes #55196. I leave it here to be merged when seen fit.

c.c. @adamsitnik

Author:teo-tsirpanis
Assignees:-
Labels:

area-System.IO

Milestone:-

teo-tsirpanis
teo-tsirpanis commented Aug 19, 2021
View reviewed changes
Comment threadsrc/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.Windows.cs
// - Net5CompatFileStreamStrategy.CompletionSource
// Async completion/return codes used by
// SafeFileHandle.OverlappedValueTaskSource
internal static class TaskSourceCodes

@teo-tsirpanisteo-tsirpanisAug 19, 2021

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

This class is unused but I didn't remove it to avoid surprising any new code that might decide to use it in the meantime until the PR is merged.

@adamsitnikadamsitnik added this to the 7.0.0 milestone Aug 19, 2021
adamsitnik
adamsitnik approved these changes Aug 19, 2021
View reviewed changes

@adamsitnikadamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

"sight for sore eyes", thank you @teo-tsirpanis !

@teo-tsirpanis

teo-tsirpanis commented Aug 19, 2021

Copy link
Copy Markdown
ContributorAuthor

It took me one Google search to understand what you said (thought there was a problem with your eyes and was confused), but I'm glad you liked it. 😊 Removing old code is satisfactory for both the doer and the beholder.

@jeffhandley

jeffhandley commented Aug 20, 2021

Copy link
Copy Markdown
Member

@adamsitnik Our plan was to do this in early 7.0 previews, but after we were sure 6.0 was stabilized and on its path toward GA. If we merge this eagerly, then any changes/fixes to the file stream strategy work might have conflicts between main and the release/6.0 branch(es). But I'll leave it to your discretion whether or not there's any risk in that. Worst case, we'd have to make a fix directly into release/6.0 that doesn't land in main first.

@adamsitnik

adamsitnik commented Aug 27, 2021

Copy link
Copy Markdown
Member

I plan to merge it after we snap for RC2 (14th of September)

@eerhardt

eerhardt commented Sep 16, 2021

Copy link
Copy Markdown
Member

@teo-tsirpanis - can you resolve the conflicts in this PR?

@teo-tsirpanis

teo-tsirpanis commented Sep 16, 2021

Copy link
Copy Markdown
ContributorAuthor

Done @eerhardt.

@teo-tsirpanis
teo-tsirpanisforce-pushed the net5compatstrategy-remove branch from d5c731f to cee7adcCompareSeptember 30, 2021 22:17
@jeffhandley
jeffhandley merged commit 3de6207 into dotnet:mainOct 9, 2021
@teo-tsirpanis
teo-tsirpanis deleted the net5compatstrategy-remove branch October 9, 2021 08:40
@adamsitnikadamsitnik mentioned this pull request Oct 16, 2021
Merged
@ghostghost locked as resolved and limited conversation to collaborators Nov 8, 2021
@adamsitnikadamsitnik added the needs-breaking-change-doc-created Breaking changes need an issue opened with https://github.com/dotnet/docs/issues/new?template=dotnet label Jan 24, 2022
@jozkee

jozkee commented Sep 28, 2022

Copy link
Copy Markdown
Member

Breaking change doc issue: dotnet/docs#31465

@jozkeejozkee removed the needs-breaking-change-doc-created Breaking changes need an issue opened with https://github.com/dotnet/docs/issues/new?template=dotnet label Sep 28, 2022
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@adamsitnikadamsitnikadamsitnik approved these changes

Assignees

No one assigned

Labels

area-System.IOcommunity-contributionIndicates that the PR has been added by a community member

Projects

None yet

Milestone

7.0.0

Development

Successfully merging this pull request may close these issues.

Remove Net5CompatFileStreamStrategy in .NET 7 early previews

5 participants

@teo-tsirpanis@jeffhandley@adamsitnik@eerhardt@jozkee