Update Autofac from 4.0.0 to 6.5.0 #317
Conversation
| @@ -16,7 +16,7 @@ | |||
| </ItemGroup> | |||
|
|
|||
| <ItemGroup> | |||
| <PackageReference Include="Autofac" Version="4.0.0" /> | |||
| <PackageReference Include="Autofac" Version="6.5.0" /> | |||
There was a problem hiding this comment.
@gritcsenko can you please explain what this upgrade accomplishes? My understanding is that this establishes the minimum supported version. So, if a particular app wants to use a newer version than that, there is no issue. Now, after this upgrade, no one can use a lower version of autofac than 6.5.0. So this limits the overall compatibility of NRules, which I would consider a downside. What’s the upside?
There was a problem hiding this comment.
Hmm... 🤔
Apparently, it was long time ago when I developed package last time.
Anyway...
Autofac v4.0.0 released with NETStandard 1.1 support (in 2016 😲). Lot of fixes was issued.
If some library targets 1.1 and uses autofac 4.0.0 it will not be able to use NRules, which targets 2.0 at minimum.
The earliest Autofac version that supports NETStandard 2.0 is 4.9.0 (see autofac/Autofac#889)
If you want to be on-par with Autofac v4.0.0 targets, NRules need to target NETStandard 1.1 too.
I agree with you that updating to 6.x or event to 5.x will break many clients.
@snikolayev Can I suggest updating to latest 4.x version (4.9.4) that has NETStandard 2.0 support?
There was a problem hiding this comment.
@gritcsenko what I can see is that Autofac 4.0.0 targets .net standard 1.0 and .net framework 4.5.1. So, an app targeting .net framework 4.6.2 would pick up NRules targeting .net standard 2.0 and Autofac targeting 4.5.1. Also, there is nothing preventing an app targeting .net standard 2.0 to target a library which itself targets .net standard 1.0, since that's just a subset of the .net standard API.
So, practically, not sure what's the benefit of bumping this reference, since it doesn't preclude apps from using a higher version of Autofac.
I guess I don't mind bumping to 4.9.4, simply given how old everything before that is.
There was a problem hiding this comment.
@snikolayev I see...
I'll close this PR and issue, since there is no benefit updating dependency. Thanks for updating my knowledge. 👍
| <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.4.0" /> | ||
| <PackageReference Include="Moq" Version="4.18.2" /> | ||
| <PackageReference Include="xunit" Version="2.4.2" /> | ||
| <PackageReference Include="xunit.runner.visualstudio" Version="2.4.5"> |
There was a problem hiding this comment.
@gritcsenko I agree it makes sense upgrading xunit and moq dependencies, but can you please do it as a separate PR, and for all test projects (as opposed to just this one)?
Closes #312