mdoc should not use TypeMap for parameter and return elements #558
Conversation
joelmartinez
left a comment
joelmartinez
left a comment
There was a problem hiding this comment.
This is great, but the same type of change also needs to be added for parameter types. Look at the MakeParameters function
joelmartinez
left a comment
joelmartinez
left a comment
There was a problem hiding this comment.
Thank you, yes this change looks good as well, and I appreciate that you added the default parameter so that any other callers who might be using GetDocParameterType will still get the original behavior.
In my haste yesterday, I asked for only this one change ... but that was partially because I was reacting to an urgent need to unblock @TianqiZhang. In the meantime, @anmeng10101 has created a preview release that includes this change so that has unblocked. So I would like to request one other change.
It's interesting that this hasn't triggered any changes in our expected integration test files ... which means that we must not have any typemap integration tests. I'd like to request that you add an integration test, along with expected XML files, that uses a typemap file, and we see the mapped type in the signature, but the original type in the return and parameter elements.
This way we can codify the behavior in our integration test suite, and better guard against regressions in the future. Thanks!
Pushing ex.expected xmls
|
|
Disable typefile name mapping
joelmartinez
left a comment
joelmartinez
left a comment
There was a problem hiding this comment.
Thanks for the great work on adding the integration testing ... really helped capture some things that otherwise would have gone unnoticed. This LGTM /cc @anmeng10101
* Updated CppWinRt formatter for enum (#561) https://ceapex.visualstudio.com/Engineering/_workitems/edit/433832 Co-authored-by: anmeng10101 <33647870+anmeng10101@users.noreply.github.com> * mdoc should not use TypeMap for parameter and return elements (#558) https://ceapex.visualstudio.com/Engineering/_workitems/edit/427756 Co-authored-by: anmeng10101 <33647870+anmeng10101@users.noreply.github.com> * Update nuget feedCredential name * update ECMAUrlParser to support nullable (?) * indent * support array scenario * fix most errors in ReachabilityTest * try another way * fix issues * add instructions about how to run jay * fix indent * add lgtm and sdl tool (#571) add LGTM and SDL checks in build pipeline * Update azure-pipelines.yml for Azure Pipelines * Update azure-pipelines.yml for Azure Pipelines * Update frameworksbootstrapper.cs * Added a debug message when unable to resolve a type (#574) Co-authored-by: anmeng10101 <33647870+anmeng10101@users.noreply.github.com> * Added support for Microsoft.UI.Xaml dependency property (#563) * Added Microsoft Xaml Dependency Property support * Trigger CI * Update AttachedEntitiesHelper.cs Co-authored-by: anmeng10101 <33647870+anmeng10101@users.noreply.github.com> * bump to 5.8.4 (#575) Co-authored-by: RanhaoXu <v-jex@microsoft.com> Co-authored-by: anmeng10101 <33647870+anmeng10101@users.noreply.github.com> Co-authored-by: Michael Jin <v-michaeljin@microsoft.com> Co-authored-by: Tianqi Zhang <tianzh@microsoft.com> Co-authored-by: Shuang Jiang <jiangshuang007@126.com> Co-authored-by: Tianqi Zhang <TianqiZhang@users.noreply.github.com>
3 participants
task detail: https://ceapex.visualstudio.com/Engineering/_workitems/edit/427756