Skip to content
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.

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

XmlSerializer: Add GenerateSerializer overload with defaultNamespace argument #57024

Closed
wants to merge 3 commits into from
Closed

XmlSerializer: Add GenerateSerializer overload with defaultNamespace argument #57024

wants to merge 3 commits into from

Conversation

TalAloni
Copy link
Contributor

@TalAloni TalAloni commented Aug 7, 2021

Motivation: In PR #46500 we called GenerateSerializerToStream directly because a GenerateSerializer overload with the defaultNamespace argument was not available, this change is proposed in order to correct that.

@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Aug 7, 2021
Add null-forgiving operator
@TalAloni TalAloni changed the title Add GenerateSerializer overload with defaultNamespace argument XmlSerializer: Add GenerateSerializer overload with defaultNamespace argument Aug 7, 2021
Minor correction
@StephenMolloy
Copy link
Member

We'll discuss this at our next team meeting. But my initial take is that this doesn't really add much at this time. In the PR that inspired this idea, I don't believe this would even help clean up that code anyway, since the code would have to check to see if it found this overload and fall back to the old method.

@TalAloni
Copy link
Contributor Author

Stephen, You are correct that this PR does not add value in the short term, but I opted to take the long term approach here.
With the help of this PR, at some point, years into the future, we will be able to clean-up the code that was added in #46500. that's the motivation here.

@StephenMolloy
Copy link
Member

We discussed this in our triage this week, and there isn't really enough value added here. The potential for versioning complexity
outweighs the benefits of this change. We're going to close this PR at this time.

@ghost ghost locked as resolved and limited conversation to collaborators Nov 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Serialization community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants