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

[release/5.0-rc2] Re-add linker attributes to generic type params of JsonSerializer methods #42189

Closed

Conversation

github-actions[bot]
Copy link
Contributor

Backport of #42186 to release/5.0-rc2

/cc @layomia

Customer Impact

Testing

Risk

@Dotnet-GitSync-Bot
Copy link
Collaborator

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@danmoseley
Copy link
Member

Needs template filling out.

@layomia
Copy link
Contributor

layomia commented Sep 15, 2020

cc @vitek-karas @MichalStrehovsky - is porting this change needed for .NET 5? These annotations were mistakenly dropped in #41484 due to attributes on generic type parameters not being supported by /t:GenerateReferenceSource (tracked in dotnet/arcade#5925).

AFAIK, the change has no impact for customers. When customers run the linker in 5.0, it will always be against the implementation assemblies.

@MichalStrehovsky
Copy link
Member

I don't think this is critical to backport - this annotation is incomplete anyway because it doesn't deal with the recursion that System.Text.Json will inavoidably do (dotnet/linker#1087 talks about that). We'll need to do something with this in .NET 6.

@vitek-karas
Copy link
Member

I agree - not critical to backport.

@layomia
Copy link
Contributor

layomia commented Sep 15, 2020

Thanks. Closing as this isn't critical for 5.0 cc @eerhardt.

@layomia layomia closed this Sep 15, 2020
@jkotas jkotas deleted the backport/pr-42186-to-release/5.0-rc2 branch September 17, 2020 12:59
@ghost ghost locked as resolved and limited conversation to collaborators Dec 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants