Skip to content

Remove implicit flush from BufferedStream.WriteByte #104822

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
Merged
jeffhandley merged 3 commits into from
Mar 23, 2025
Merged

Remove implicit flush from BufferedStream.WriteByte #104822

jeffhandley merged 3 commits into from
Mar 23, 2025

Conversation

@ANahr
Copy link
Contributor

@ANahr ANahr commented Jul 12, 2024

Remove the inconsistent implicit flush from BufferedStream.WriteByte, so that all BufferedStream.Write* Methods behave identically.

Fix #104559

@ghost ghost added the area-System.IO label Jul 12, 2024
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Jul 12, 2024
This was referenced Jul 13, 2024
Open
Closed
@ANahr
Copy link
Contributor Author

ANahr commented Jul 14, 2024

Failures seem to be unrelated

@adamsitnik adamsitnik self-assigned this Jul 15, 2024
adamsitnik
adamsitnik reviewed Jul 15, 2024
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.

Thank you for your contribution @ANahr !

Before I review the PR we need to discuss whether we want to take this breaking change. Let's continue the discussion in #104559 (comment)

@adamsitnik adamsitnik added this to the Future milestone Jul 15, 2024
@adamsitnik adamsitnik added the breaking-change Issue or PR that represents a breaking API or functional change over a previous release. label Jul 15, 2024
@dotnet-policy-service dotnet-policy-service bot added the needs-breaking-change-doc-created Breaking changes need an issue opened with https://github.com/dotnet/docs/issues/new?template=dotnet label Jul 15, 2024
@ANahr
Copy link
Contributor Author

ANahr commented Jul 23, 2024 •
edited
Loading

@adamsitnik Is there anything I could do for this PR?
Imho the linked issue is resolved.
I could create the breaking change documentation but am not sure which version I would have to select.

@ANahr
Copy link
Contributor Author

ANahr commented Aug 7, 2024

@adamsitnik Any updates? Can this be fixed for .Net 9?

@ANahr
Copy link
Contributor Author

ANahr commented Oct 31, 2024 •
edited
Loading

@adamsitnik Ping - The issue has been open for months. The linked issue is resolved. What is left to do?

jeffhandley
jeffhandley approved these changes Mar 23, 2025
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.

Sorry for the delays on this, @ANahr. Thank you for the details and effort you provided on this. I've merged main into your branch. Once green, we will merge for this to be part of .NET 10 as a breaking change.

This was referenced Mar 23, 2025
Open
Open
Open
Closed
Open
Closed
@jeffhandley
Copy link
Member

jeffhandley commented Mar 23, 2025

All failures are known and unrelated.

@jeffhandley jeffhandley merged commit 6fda811 into dotnet:main Mar 23, 2025
134 of 140 checks passed
@AndyAyersMS AndyAyersMS mentioned this pull request Mar 25, 2025
Closed
@BruceForstall BruceForstall mentioned this pull request Apr 3, 2025
Closed
@github-actions github-actions bot locked and limited conversation to collaborators Apr 23, 2025
@ericstj
Copy link
Member

ericstj commented Oct 3, 2025

📋 Breaking Change Documentation Required

Create a breaking change issue with AI-generated content

Generated by Breaking Change Documentation Tool - 2025-10-03 14:09:36

@ericstj
Copy link
Member

ericstj commented Oct 22, 2025

@jeffhandley @adamsitnik could you help create the breaking change doc for this? Just try the above link and review the content.

@jeffhandley
Copy link
Member

jeffhandley commented Oct 22, 2025 •
edited
Loading

@ericstj Done -- that worked marvelously; I only needed to add one extra statement to what it produced.

(dotnet/docs#49418) [Breaking change]: Remove implicit flush from BufferedStream.WriteByte

@jeffhandley jeffhandley removed the needs-breaking-change-doc-created Breaking changes need an issue opened with https://github.com/dotnet/docs/issues/new?template=dotnet label Oct 22, 2025
@jeffhandley jeffhandley modified the milestones: Future, 10.0.0 Oct 22, 2025
@adamsitnik
Copy link
Member

adamsitnik commented Oct 23, 2025

@jeffhandley @adamsitnik could you help create the breaking change doc for this? Just try the above link and review the content.

I've created it two weeks ago after reading your email:

dotnet/docs#48913
dotnet/docs#49123

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

Reviewers

@adamsitnik adamsitnik adamsitnik left review comments

@jeffhandley jeffhandley jeffhandley approved these changes

Assignees

@adamsitnik adamsitnik

Labels

area-System.IO breaking-change Issue or PR that represents a breaking API or functional change over a previous release. community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Milestone

10.0.0

Development

Successfully merging this pull request may close these issues.

Unexpected and undocumented (performance) pitfall in BufferedStream.WriteByte

4 participants

@ANahr @jeffhandley @ericstj @adamsitnik