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 an assembly targeting .NET 4.5 #1108
Conversation
{ | ||
Approvals.Verify(PublicApiGenerator.GetPublicApi(typeof(A).Assembly)); | ||
ApproveApi("net45"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know, it's a little less efficient than just using the assembly that's in memory. I can revert, if you like.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it's fine; it's better to do it the same way for all assemblies, when possible. But the project reference to FakeItEasy should be removed from this project, since it's no longer necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Of course!
<!-- .NET 4.5 --> | ||
|
||
<PropertyGroup Condition="'$(TargetFramework)' == 'net45'"> | ||
<DefineConstants>$(DefineConstants);FEATURE_BINARY_SERIALIZATION;FEATURE_REFLECTION_GETASSEMBLIES;FEATURE_SELF_INITIALIZED_FAKES</DefineConstants> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note the lack of FEATURE_EVENT_ARGS_MUST_EXTEND_EVENTARGS
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @blairconrad. Looks good to me, just one minor comment.
{ | ||
Approvals.Verify(PublicApiGenerator.GetPublicApi(typeof(A).Assembly)); | ||
ApproveApi("net45"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it's fine; it's better to do it the same way for all assemblies, when possible. But the project reference to FakeItEasy should be removed from this project, since it's no longer necessary.
It was a good comment. I've removed the reference and the superfluous "content" entries from the csproj. |
Thanks, @blairconrad 👍 |
Fixes #1008.
I know we're a little concerned about the lack of testing for the .NET 4.0 assembly, and how having both 4.0 and 4.5 may lead to extra maintenance, but there's nothing preventing us from removing 4.0 support in the future.