jsakamoto / ipaddressrange

.NET Class Library for range of IP address, both IPv4 and IPv6.
Mozilla Public License 2.0
368 stars 71 forks source link

Support more .NET and .NET standard versions #79

Closed TETYYS closed 1 year ago

TETYYS commented 1 year ago

Added more .NET Standard versions and .NET 6 as targets while also updating referenced packages in test project, one of them was deprecated.

MSTest refused to run test files that did not have namespaces so I also added those.

TETYYS commented 1 year ago

Don't know why but appveyor keeps using old image for testing which doesn't support .NET 6

jsakamoto commented 1 year ago

@TETYYS Thank you for your contributions!

By the way, I'm not sure what is the meaning of adding variations of the target framework. .NET Standard is merely the specification that the various .NET runtimes should be compliant with. So, that the IPAddressRange library requires .NET Standard 1.4 means .NET 6 is valid for that condition to use the library. The figure below shows the target platforms for which IPAddressRange is available, as seen on nuget.org.

image https://www.nuget.org/packages/IPAddressRange#supportedframeworks-body-tab

Therefore, I can only accept your pull request if there is an advantage to adding variations to the target framework. Could you explain me in more detail of this point?

TETYYS commented 1 year ago

Installing .netstandard 1.4 packages adds a dependency on NETStandard.Library 1.6.1 which in turn adds dependencies to wide variety of different libraries like System.IO.Compression.ZipFile or System.XML.XDocument, which I would like to avoid depending on. .netstandard 2.0 doesn't require any of that.

jsakamoto commented 1 year ago

@TETYYS In my understanding, those wide varieties of dependencies will be type forwarded, so you might not be worried about that. I verified that with a simple Blazor WebAssembly app.

There was no difference in the published contents of the Blazor app between the cases of referencing the IPAddressRange library that the target framework is .NET Standard 1.4 only edition and the target framework is .NET Standard to .NET 6.

Does that make sense? If so, I would like to ask you to revert the change only about the target frameworks declaration in the IPAddressRange.csproj.

Anyway, updating the unit test project is your great work. I'm going to accept your pull request only for the updating unit test project part for now. I really appreciate your contributions! 👍

TETYYS commented 1 year ago

Hm, you're right. It just looks bad on surface.

Appveyor has still not updated to 2022 VS and I don't have much experience with it to fix it

jsakamoto commented 1 year ago

Appveyor has still not updated to 2022 VS and I don't have much experience with it to fix it

Lots of thanks for being aware of that! Please don't mind about that because I'll handle it. 😊

jsakamoto commented 1 year ago

@TETYYS I have considered your pull request again. The .NET Standard 1.4 dependency is enough to run on recent .NET versions such as .NET 6. However, I can understand that the NuGet package manager GUI on Visual Studio apparently looks awful.

Screenshot 2022-12-14 204313

So, I began to think that it is not a bad idea to add other minimum required .NET Standard dependencies to this library to improve looking of the NuGet package manager GUI. To do that, adding just only the .NET Standard 2.1 dependency looks good. After doing that, the NuGet package manager GUI on a .NET 6 project shows extremely simple dependencies on its window.

Screenshot 2022-12-14 210659

There is no need to add dependencies of specific .NET runtime versions such as "net6.0", "net7.0", and so on.

In conclusion, I'm going to merge this pull request, but moreover, I'll change the target frameworks to support only netstandard1.4, netstandard2.1, and net452 after merging. Finally, I'll publish the new version of this library to nuget.org.

Again, thank you for your contributions!

jsakamoto commented 1 year ago

@TETYYS

I published the new version of this library as ver.5.

Thanks!