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

Fix XmlSerializer to work on NetTcp/Duplex MEP #344

Merged
merged 1 commit into from
Sep 22, 2015

Conversation

khdang
Copy link
Member

@khdang khdang commented Sep 18, 2015

Fix #292

@dnfclas
Copy link

dnfclas commented Sep 18, 2015

Hi @khdang, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by .NET Foundation and real humans are currently evaluating your PR.

TTYL, DNFBOT;

dispatch.SerializeReply = _reflector.ReplyRequiresSerialization;
}

if (_reflector.Attribute.SupportFaults && !dispatch.IsFaultFormatterSetExplicit)
Copy link
Member Author

Choose a reason for hiding this comment

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

This is ported from Desktop code. I've removed the following because wrapper is not used at all:

else
{
    var wrapper = dispatch.FaultFormatter as IDispatchFaultFormatterWrapper;
    if (wrapper != null)
    {
        wrapper.InnerFaultFormatter = (IDispatchFaultFormatter)CreateFaultFormatter(dispatch.FaultContractInfos);
    }
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @khdang -- just a question. What does it mean when you say "the wrapper is not used at all". Does that mean we are not currently using that code path or that it is impossible to execute that code path in NET Core?

Copy link
Member Author

Choose a reason for hiding this comment

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

I mean that code path doesn't seem to have any effect since wrapper is not used anywhere else after it is initialized.

Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like IDispatchFaultFormatterWrapper isn't even defined in our code base. It looks like it was invented for the WebFaultFormatter in Desktop, so I think you're right it is not needed. Thanks. LGTM.

@roncain
Copy link
Contributor

roncain commented Sep 18, 2015

LGTM subject to one question in the comments above

@iamjasonp
Copy link
Member

LGTM.

However, prior to merge, can you fix the commit message and author? Your commit doesn't seem to be resolving to your GitHub account.

@khdang khdang changed the title Fix #292, XmlSerializer does not working on NetTcp/Duplex MEP Fix XmlSerializer does not working on NetTcp/Duplex MEP Sep 21, 2015
@khdang khdang changed the title Fix XmlSerializer does not working on NetTcp/Duplex MEP Fix XmlSerializer to work on NetTcp/Duplex MEP Sep 21, 2015
@khdang khdang force-pushed the fix292_xmlserializerformat branch from 30546cb to 5593c71 Compare September 21, 2015 17:48
@khdang khdang force-pushed the fix292_xmlserializerformat branch from 5593c71 to 437a48f Compare September 21, 2015 22:14
iamjasonp added a commit that referenced this pull request Sep 22, 2015
Fix XmlSerializer to work on NetTcp/Duplex MEP
@iamjasonp iamjasonp merged commit 9913fea into dotnet:master Sep 22, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants