Skip to content

Revert "Disable Net5CompatFileStream by default (#50166)" #51142

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.

Sign up for GitHub

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

Jump to bottom
Closed
carlossanlop wants to merge 2 commits into from
Closed

Revert "Disable Net5CompatFileStream by default (#50166)" #51142

carlossanlop wants to merge 2 commits into from

Conversation

@carlossanlop
Copy link
Contributor

@carlossanlop carlossanlop commented Apr 12, 2021

Partially addresses #51141

Unblocks dotnet/sdk#16810

This reverts commit 8b2ad5b to unblock dotnet/sdk.

@carlossanlop
Revert "Disable Net5CompatFileStream by default (#50166)"
f7c1d86 
This reverts commit 8b2ad5b to unblock dotnet/sdk.
@carlossanlop carlossanlop added this to the 6.0.0 milestone Apr 12, 2021
@carlossanlop carlossanlop self-assigned this Apr 12, 2021
@ghost
Copy link

ghost commented Apr 12, 2021

Tagging subscribers to this area: @carlossanlop
See info in area-owners.md if you want to be subscribed.

Issue Details

Partially addresses #51141

Unblocks dotnet/sdk#16810

This reverts commit 8b2ad5b to unblock dotnet/sdk.

Author: carlossanlop
Assignees: carlossanlop
Labels:

area-System.IO
Milestone: 6.0.0

@carlossanlop carlossanlop mentioned this pull request Apr 12, 2021
Merged
adamsitnik
adamsitnik approved these changes Apr 12, 2021
View reviewed changes
Copy link
Member

@adamsitnik adamsitnik left a comment

Choose a reason for hiding this comment

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

@carlossanlop thank you for doing this! I hope to have a bug fix tomorrow

jeffhandley
jeffhandley approved these changes Apr 12, 2021
View reviewed changes
Copy link
Member

@jeffhandley jeffhandley left a comment

Choose a reason for hiding this comment

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

Thanks, @carlossanlop. Out of curiosity, could we alternatively enable the compat switch in the SDK project(s) to unblock that build? Or do you think there are too many projects affected by the bug right now?

@carlossanlop
Copy link
Contributor Author

carlossanlop commented Apr 12, 2021 •
edited
Loading

Out of curiosity, could we alternatively enable the compat switch in the SDK project(s) to unblock that build? Or do you think there are too many projects affected by the bug right now?

I am unsure of the impact on other repos. We think it's safer at this point having this change reverted, do our tests again, make sure the issue is reproduced locally, and get it fixed before we turn on the compat switch by default again.

@jeffhandley
Copy link
Member

jeffhandley commented Apr 12, 2021

Makes sense; thanks!

@adamsitnik
Copy link
Member

adamsitnik commented Apr 12, 2021

I have a fix, please don't merge this PR ;)

@adamsitnik
Copy link
Member

adamsitnik commented Apr 12, 2021

Fix of the actual bug: #51151

@carlossanlop carlossanlop added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Apr 12, 2021
@carlossanlop
Copy link
Contributor Author

carlossanlop commented Apr 12, 2021

Thanks Adam! If the CI in your PR finishes successfully, we will merge that. Otherwise, we'll merge this PR.

@carlossanlop carlossanlop deleted the RevertNet5CompatFS branch April 13, 2021 06:57
@ghost ghost locked as resolved and limited conversation to collaborators May 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@jeffhandley jeffhandley jeffhandley approved these changes

@adamsitnik adamsitnik adamsitnik approved these changes

@jozkee jozkee jozkee approved these changes

Assignees

@carlossanlop carlossanlop

Labels

area-System.IO NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons)

Projects

None yet

Milestone

6.0.0

Development

Successfully merging this pull request may close these issues.

4 participants

@carlossanlop @jeffhandley @adamsitnik @jozkee