Skip to content

Remove implicit flush from BufferedStream.WriteByte#104822

Merged
jeffhandley merged 3 commits into
dotnet:mainfrom
ANahr:removeimplicitflush
Mar 23, 2025
Merged

Remove implicit flush from BufferedStream.WriteByte#104822
jeffhandley merged 3 commits into
dotnet:mainfrom
ANahr:removeimplicitflush

Conversation

@ANahr

@ANahr ANahr commented Jul 12, 2024

Copy link
Copy Markdown
Contributor

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

ANahr commented Jul 14, 2024

Copy link
Copy Markdown
Contributor Author

Failures seem to be unrelated

@adamsitnik adamsitnik self-assigned this Jul 15, 2024
adamsitnik
adamsitnik reviewed Jul 15, 2024
View reviewed changes

@adamsitnik adamsitnik 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.

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

ANahr commented Jul 23, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@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

ANahr commented Aug 7, 2024

Copy link
Copy Markdown
Contributor Author

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

@ANahr

ANahr commented Oct 31, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@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

@jeffhandley jeffhandley 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.

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

jeffhandley commented Mar 23, 2025

Copy link
Copy Markdown
Member

All failures are known and unrelated.

@jeffhandley jeffhandley merged commit 6fda811 into dotnet:main Mar 23, 2025
@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

ericstj commented Oct 3, 2025

Copy link
Copy Markdown
Member

📋 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

ericstj commented Oct 22, 2025

Copy link
Copy Markdown
Member

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

@jeffhandley

jeffhandley commented Oct 22, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

@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

adamsitnik commented Oct 23, 2025

Copy link
Copy Markdown
Member

@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

@ANahr ANahr deleted the removeimplicitflush branch June 12, 2026 16:04
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