Enable IAsyncEnumerable for netframework #1131
Conversation
| @@ -45,9 +45,11 @@ | |||
| <ItemGroup Condition=" '$(TargetFrameworkIdentifier)' == '.NETFramework' "> | |||
| <PackageReference Include="Microsoft.NETFramework.ReferenceAssemblies" Version="1.0.3" PrivateAssets="All" /> | |||
| <PackageReference Include="System.Threading.Tasks.Extensions" Version="4.6.3" /> | |||
| <PackageReference Include="Microsoft.Bcl.AsyncInterfaces" Version="9.0.6" /> | |||
There was a problem hiding this comment.
Some thoughts on this
Since this is to enable a single extension method that can be written by the user, maybe it would make sense to forgo adding the Bcl.AsyncInterfaces and let the user create the extension method when needed? Is there any downside adding this dependency that are currently overlooked?
Alternatively, adding this would make it easier to add the IAsyncEnumerable extensions methods on more sockets/use cases.
Personally, I added a few extensions methods with IAsyncEnumerable in my projects but their usecases are probably such that I wouldn't use/depend on those offered by NetMQ.
There was a problem hiding this comment.
Interesting.
My view: The idea of this pull request is to remove a difference between NET and NETFramework. The additional dependency is only added for NETFramework.
Some addition info: https://stackoverflow.com/questions/56651472/does-c-sharp-8-support-the-net-framework
There was a problem hiding this comment.
There's no clear right answer here. Dependencies shouldn't be added lightly, but the commonality between targets has real value.
If a user on framework upgrades but doesn't ship the new DLL, things will work correctly so long as they don't call these methods, so I think that's ok.
Let's add this. It does seem to simplify things overall.
No description provided.