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

added [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicFields)] annotations to generic enums #330

Merged
merged 1 commit into from
Oct 10, 2022

Conversation

filipw
Copy link
Contributor

@filipw filipw commented Oct 6, 2022

At the moment the build fails when using .NET SDK 7.0.100-rc.1
There is a trimming error:

/fido2-net-lib/Src/Fido2.Models/Converters/EnumNameMapper.cs(68,35): error IL2090: 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicFields' in call to 'System.Type.GetFields(BindingFlags)'. The generic parameter 'TEnum' of 'Fido2NetLib.EnumNameMapper' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to. [/Users/filipw/Documents/dev/fido2-net-lib/Src/Fido2.Models/Fido2.Models.csproj::TargetFramework=net6.0]

This PR adds the necessary annotations.

@codecov-commenter
Copy link

Codecov Report

Merging #330 (a3f0e69) into master (1eda2a3) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #330   +/-   ##
=======================================
  Coverage   78.09%   78.09%           
=======================================
  Files          87       87           
  Lines        2433     2433           
  Branches      401      401           
=======================================
  Hits         1900     1900           
  Misses        431      431           
  Partials      102      102           
Impacted Files Coverage Δ
Src/Fido2.Models/Converters/EnumNameMapper.cs 100.00% <ø> (ø)
Src/Fido2.Models/Converters/FidoEnumConverter.cs 66.66% <ø> (ø)
Src/Fido2/Extensions/EnumExtensions.cs 100.00% <ø> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@abergs abergs self-requested a review October 10, 2022 12:28
@abergs
Copy link
Collaborator

abergs commented Oct 10, 2022

Thank you @filipw. Looks good, will do a quick review and build/test on my machine before merging.

@abergs abergs merged commit 753f48a into passwordless-lib:master Oct 10, 2022
@abergs
Copy link
Collaborator

abergs commented Oct 10, 2022

@filipw Every looks good. Merging. Where can I read more about the changes in .net 7 that causes these trimming issues?

@abergs abergs added the enhancement Enhancements or general improvements label Oct 10, 2022
@filipw
Copy link
Contributor Author

filipw commented Oct 26, 2022

There is some documentation around what to do in the library to prepare it for trimming here https://learn.microsoft.com/en-us/dotnet/core/deploying/trimming/prepare-libraries-for-trimming and it includes some info around these attributes. These are good practices to follow anyhow, but I am not sure why it suddenly showed up as error with .NET 7 SDK 🤷‍♂️

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancements or general improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants