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

Add the approved span marshallers and remove the v1 model #71989

Merged
merged 8 commits into from
Jul 12, 2022

Conversation

jkoritzinsky
Copy link
Member

This PR builds on #71978 and adds the approved span marshaller shapes. Since we will no longer have usage of the v1 marshalling model, this PR also removes it entirely.

This PR would replace #71980, but that one should be merged if this one isn't ready by P7. If this PR misses P7, we need to add type forwarders for the types moved between the ref assemblies.

In combination with the referenced PRs, this PR fixes #70859

Fixes #69281

@jkoritzinsky jkoritzinsky added area-System.Runtime.InteropServices blocked Issue/PR is blocked on something - see comments source-generator Indicates an issue with a source generator feature labels Jul 12, 2022
@ghost
Copy link

ghost commented Jul 12, 2022

Tagging subscribers to this area: @dotnet/interop-contrib
See info in area-owners.md if you want to be subscribed.

Issue Details

This PR builds on #71978 and adds the approved span marshaller shapes. Since we will no longer have usage of the v1 marshalling model, this PR also removes it entirely.

This PR would replace #71980, but that one should be merged if this one isn't ready by P7. If this PR misses P7, we need to add type forwarders for the types moved between the ref assemblies.

In combination with the referenced PRs, this PR fixes #70859

Fixes #69281

Author: jkoritzinsky
Assignees: -
Labels:

area-System.Runtime.InteropServices, blocked, source-generator

Milestone: -

@dotnet-issue-labeler
Copy link

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@stephentoub
Copy link
Member

stephentoub commented Jul 12, 2022

@jkoritzinsky, so once this PR is in, we can start using spans (unadorned) in LibraryImport methods?

@jkoritzinsky
Copy link
Member Author

@stephentoub Yes! I'd update places to use this now, but I want to try to get this in for P7 if possible.

…upport this and it was breaking the span marshallers.
You served us well, but your time has passed.
@jkoritzinsky jkoritzinsky removed the blocked Issue/PR is blocked on something - see comments label Jul 12, 2022
@jkoritzinsky jkoritzinsky marked this pull request as ready for review July 12, 2022 04:52
@jkoritzinsky
Copy link
Member Author

The failing test should be deleted. I have a commit locally. That deletes the test, but I'm going to let CI keep running to see if there's anything else that fails. If this is the only failure, I'll push up the change to delete the test and then merge the PR.

That way we don't get this PR stuck waiting for machines to run tests when we want to get it in before P7 branches off.

@jkoritzinsky
Copy link
Member Author

Okay all of the test failures are the expected failure.

@jkoritzinsky jkoritzinsky merged commit 5e94a46 into dotnet:main Jul 12, 2022
@jkoritzinsky jkoritzinsky deleted the span-marshallers branch July 12, 2022 23:35
@ghost ghost locked as resolved and limited conversation to collaborators Aug 12, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
4 participants