search

yellowfeather / DbfDataReader

DbfDataReader is a small fast .Net Core library for reading dBase, xBase, Clipper and FoxPro database files
MIT License
134 stars 61 forks source link

Check if the package references for the supported target frameworks are correct #214

Closed Balkoth closed 1 year ago

Balkoth commented 1 year ago

Is your feature request related to a problem? Please describe. I am not sure if the project references of the nuget package are setup correctly, because there is no choice made whether the net6.0 or the netstandard2.1 framework is used:

<ItemGroup>
  <PackageReference Include="Microsoft.SourceLink.GitHub" Version="1.1.1" PrivateAssets="All" />
  <PackageReference Include="MinVer" Version="4.3.0" PrivateAssets="All" />
  <PackageReference Include="System.Data.Common" Version="4.3.0" />
  <PackageReference Include="System.Memory" Version="4.5.5" />
  <PackageReference Include="System.Text.Encoding.CodePages" Version="7.0.0" />
</ItemGroup>

Are the packages System.Data.Common and System.Memory really needed when net6.0 is the target framework?

Describe the solution you'd like If they are not needed i would like to see the project references setup like this:

<ItemGroup>
  <PackageReference Include="Microsoft.SourceLink.GitHub" Version="1.1.1" PrivateAssets="All" />
  <PackageReference Include="MinVer" Version="4.3.0" PrivateAssets="All" />
  <PackageReference Include="System.Text.Encoding.CodePages" Version="7.0.0" />
</ItemGroup>

<ItemGroup Condition="'$(TargetFramework)' == 'netstandard2.1'">
  <PackageReference Include="System.Data.Common" Version="4.3.0" />
  <PackageReference Include="System.Memory" Version="4.5.5" />
</ItemGroup>

Additional context In a .net7.0 project referencing the package DbfDataReader this changes the transitive packages from this: image to this: image

Balkoth commented 1 year ago

I don't know if System.Text.Encoding.CodePages is needed either. According to this https://github.com/dotnet/runtime/issues/76102#issuecomment-1256709609 post that library should be part of the shared runtime if using net6.0 or later.

<ItemGroup>
  <PackageReference Include="Microsoft.SourceLink.GitHub" Version="1.1.1" PrivateAssets="All" />
  <PackageReference Include="MinVer" Version="4.3.0" PrivateAssets="All" />
</ItemGroup>

<ItemGroup Condition="'$(TargetFramework)' == 'netstandard2.1'">
  <PackageReference Include="System.Data.Common" Version="4.3.0" />
  <PackageReference Include="System.Memory" Version="4.5.5" />
  <PackageReference Include="System.Text.Encoding.CodePages" Version="7.0.0" />
</ItemGroup>
chrisrichards commented 1 year ago

Thanks for pointing this out, fixed!