Skip to content

Revert changes to EncoderNLS/DecoderNLS.Convert#752

Merged
jkotas merged 1 commit intodotnet:masterfrom
GrabYourPitchforks:revert_nls
Dec 27, 2019
Merged

Revert changes to EncoderNLS/DecoderNLS.Convert#752
jkotas merged 1 commit intodotnet:masterfrom
GrabYourPitchforks:revert_nls

Conversation

@GrabYourPitchforks
Copy link
Member

Fixes #594.

This restores the original completed output parameter behavior that was present in Full Framework and .NET Core 2.x, reverting the changes that were introduced in the .NET Core 3.0 / 3.1 timeframe. I've also added a code comment explaining the contract of the completed parameter for the Convert routines. That comment can serve as the basis for updated API documentation text.

This PR is for .NET 5.0 specifically. After approval there will be separate PRs for .NET Core 3.1.1 (downlevel).

Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

Thanks!

@GrabYourPitchforks
Copy link
Member Author

I was seeing some unexpected System.Private.Xml test failures on my machine, but I don't think the failing tests were related to this code change. Giving CI a shot to see if things pass there.

@GrabYourPitchforks GrabYourPitchforks added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Dec 11, 2019
@GrabYourPitchforks
Copy link
Member Author

Temporarily marking as no merge because it's dependent on #494 (live-local builds). Once that goes through, this can go through.

@jkotas jkotas closed this Dec 13, 2019
@jkotas jkotas reopened this Dec 13, 2019
@jkotas jkotas added area-System.Text.Encoding and removed NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) area-System.Runtime labels Dec 13, 2019
@jkotas jkotas closed this Dec 17, 2019
@jkotas jkotas reopened this Dec 17, 2019
@jkotas
Copy link
Member

jkotas commented Dec 17, 2019

/azp run runtime-libraries

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jkotas jkotas merged commit d49b541 into dotnet:master Dec 27, 2019
@GrabYourPitchforks GrabYourPitchforks deleted the revert_nls branch January 14, 2020 22:28
@ghost ghost locked as resolved and limited conversation to collaborators Dec 11, 2020
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.

Compat: Revert behavior of DecoderNLS / EncoderNLS.Convert to match Full Framework

4 participants