-
Notifications
You must be signed in to change notification settings - Fork 558
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
Implement Mtom message encoding #4687
Conversation
Looks like tests failed because the test server had the wrong code installed. I suspect this might have been another test running in parallel. Trying again |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple of questions. But I didn't see anything alarming. Looked quite a bit like NetFX code in most places. ;)
src/System.Private.ServiceModel/src/Internals/System/Xml/XmlConverter.cs
Show resolved
Hide resolved
src/System.Private.ServiceModel/src/Internals/System/Xml/XmlExceptionHelper.cs
Outdated
Show resolved
Hide resolved
@@ -17,7 +16,7 @@ namespace System.ServiceModel.Channels | |||
// If any of the const's are modified in this file, they must also be modified | |||
// on the Internal.ServiceModel.Primitives contract and implementation assemblies. | |||
|
|||
public static class EncoderDefaults | |||
internal static class EncoderDefaults |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Were these various EncoderDefaults public in a previous release? It looks like they've been public for a while, but not exposed in the ref assembly. I see they were not public in 4.8, but is there any concern bringing them back to internal?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They shouldn't have been public in the first place. This is just some additional cleanup.
Fixes #1810