Skip to content

Conversation

@tdykstra
Copy link
Contributor

@tdykstra tdykstra commented May 11, 2020

Fixes #16690

cc @ahsonkhan

@steveharter
Copy link
Contributor

@layomia do we have an issue to round-trip stack "properly" for 5.0?

Copy link
Member

@BillWagner BillWagner left a comment

Choose a reason for hiding this comment

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

This LGTM @tdykstra. You can :shipit: when ready.

When would you want to move these snippets into the new location?

@tdykstra
Copy link
Contributor Author

When would you want to move these snippets into the new location?

I think the diff would be too confusing to do it in the same PR as an update to the code, so I'll do it separately, will try to get to it this week.

@layomia
Copy link
Contributor

layomia commented May 13, 2020

do we have an issue to round-trip stack "properly" for 5.0?

@steveharter
We did, but closed it because there's no standard way to fix the bug. Our suggested way (as shown in the doc example in this PR) is to reverse the items on serialization, but one could argue that the items should be reversed on deserialization - dotnet/runtime#31211 (comment). We can reexamine the decision if we feel strongly that our suggested way is correct/adequate for most scenarios.

Copy link
Contributor

@ahsonkhan ahsonkhan left a comment

Choose a reason for hiding this comment

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

Otherwise, looks good.

@tdykstra tdykstra merged commit f9a9d7c into dotnet:master May 14, 2020
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.

Add Stack<T> converter example

6 participants