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

Update NonVersionableAttribute attribute trimming #48213

Merged
merged 1 commit into from
Feb 17, 2021

Conversation

marek-safar
Copy link
Contributor

No description provided.

@dotnet-issue-labeler
Copy link

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.

@MichalStrehovsky
Copy link
Member

More of a general question, but since you're touching this file - should we move this to the shared location and start making these conditional on feature switches? For example the entry for System.ObsoleteAttribute at the beginning has comments that this might be sketchy, but it's an okay default for WASM. In .NET 6, Mono's CoreLib won't be WASM only anymore.

Would be nice to have them as feature switches. Blazor/WASM can default those to on by default. Xamarin can also make their own choices. Desktop workloads would probably default them all to "kept", but it's a nice opt-in to have.

@marek-safar
Copy link
Contributor Author

I'm not against someone porting everything or a subset of this to a shared location but it'd be better done separately. Right now any Mono workload defaults to these settings without feature switches.

@MichalStrehovsky
Copy link
Member

Ok, I filed #48217. I put it as Milestone 6.0.0 because we likely don't want to ship a version of Xamarin where Entity Framework doesn't work right after trimming.

@marek-safar marek-safar merged commit 49cb926 into dotnet:master Feb 17, 2021
@marek-safar marek-safar deleted the attrs2 branch February 17, 2021 09:34
@ghost ghost locked as resolved and limited conversation to collaborators Mar 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants