-
Notifications
You must be signed in to change notification settings - Fork 130
MS Config and DependencyModel refs updated #96
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
Conversation
I've never used Travis, not sure why it blows up with a Also, didn't bump the version, not sure whether you'd consider this a minor or patch change.
|
@adamchester Thanks! The file isn't in the SLN view and I didn't think to jump back to the file view. All good now, much appreciated. |
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.
LGTM! Just wondering if we need to define APPDOMAIN for the netstandard2.0 target for completeness, now? Or if we can remove it entirely? (Not sure why we use it in this lib.)
<ItemGroup Condition="'$(TargetFramework)' == 'netstandard2.0'"> | ||
<PackageReference Include="Microsoft.Extensions.Configuration.Abstractions" Version="2.0.1" /> | ||
</ItemGroup> | ||
|
||
<PropertyGroup Condition=" '$(TargetFramework)' == 'net451' "> | ||
<DefineConstants>$(DefineConstants);APPDOMAIN</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.
Are the features behind #if APPDOMAIN
now usable on .NET Standard 2.0?
Since APPDOMAIN was specific to To be honest, my focus in this change was updating MS Config to v2, so as long as the DependencyModel stuff built and tested clean, I figured it was good. But I'm happy to expand on the PR if you think something else is needed. |
Ah, great 👍 - that code enables using types without listing all of the assemblies in |
@nblumhardt Good call, you're right. I'd moved Not sure why I thought |
👍 We might have to see how the combined Framework 4.5.1/Standard 2.0 targeting works out in practice - have had some issues with this kind of setup in the past; shouldn't be any trouble dropping 4.5.1 support from 3.0.0 of this package if we end up having to. Thanks for your efforts, @MV10 ! |
Per #83, updated and tested references to
Microsoft.Extensions.DependencyModel
and
Microsoft.Extensions.Configuration.*
. Updated .NET Standard from 1.6 to 2.0.Confirmed both framework sample versions also run correctly. In all project files, old .NET Framework and .NET Standard refs were updated separately (conditional TFMs) to latest supporting versions. Also added a comment to the test project explaining how to run the multi-targeted tests from the command line (it was new to me, I suspect it is not commonly known).