Skip to content
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

Use .NET Standard 2.0 CommandLineParser, Update to net461 for NS2.0 support #992

Merged
merged 2 commits into from
Jan 13, 2019
Merged

Use .NET Standard 2.0 CommandLineParser, Update to net461 for NS2.0 support #992

merged 2 commits into from
Jan 13, 2019

Conversation

glennawatson
Copy link
Contributor

  • Added support for CommandLineParser 2.3.1 Net Standard 2.0 library -- this is for future work on support Xamarin releases
  • Now use NET461 for the libraries, this is required for .Net Standard 2.0 support.
  • Now use $(TargetFramework.StartsWith('net4')) in our csproj files, this allows upgrades to .NET 4 based libraries without the pain currently required.

@adamsitnik
Copy link
Member

Hi @glennawatson

Thanks for the PR!

I am fine with moving from CommandLineParser to CommandLineParser.NS20 but I don't like the net46 -> net461 update.

Could you please provide a link to the issue that we try to solve? I want to get more insight into it.

I also want to switch from CommandLineParser to https://github.com/dotnet/command-line-api, but it's not available on NuGet.org yet (dotnet/command-line-api#356)

@glennawatson
Copy link
Contributor Author

@adamsitnik you can't compile .net standard 2.0 libraries in your application with net46.

See https://blogs.msdn.microsoft.com/dotnet/2017/08/14/announcing-net-standard-2-0/
Broad platform support. .NET Standard 2.0 is supported on the following platforms:
.NET Framework 4.6.1
.NET Core 2.0
Mono 5.4
Xamarin.iOS 10.14
Xamarin.Mac 3.8
Xamarin.Android 7.5
Upcoming version of UWP (expected to ship later this year)

So if you want to use either th command line parser ns2 or th command line API your going to hit the same wall you need to upgrade to net461 otherwise you'll get compile errors.

@glennawatson
Copy link
Contributor Author

commandlineparser/commandline#292 is the issue I am hitting. It's solved with commandlineparser/commandline#307 but again you can't get that fix without going to net461 due to it being the first version to support net standard 2.0. Net46 is not supported in that scenario

@ViktorHofer
Copy link
Member

Just as a note, CommandLineParser.NS20 was published by me some months ago to overcome the time till my PR is accepted. Unfortunately that still hasn't happened.

@adamsitnik
Copy link
Member

@AndreyAkinshin are you ok with the update from .NET 4.6 to 4.6.1?

I think that we will have to do it anyway since Roslyn's next version is going to support .NET Standard 2.0 only: https://www.nuget.org/packages/Microsoft.CodeAnalysis.CSharp/2.11.0-beta1-final

(the lowest .NET supporting .NET Standard 2.0 is .NET 4.6.1)

@AndreyAkinshin
Copy link
Member

In general, I don't like the .NET 4.6->4.6.1 update. However, I understand that it will simplify future development a lot. Let's do it.

In the future, we can introduce the BenchmarkDotNet.Annotations NuGet package (we already discussed this idea before) with low runtime requirements (e.g., .NETStandard 1.0; .NET Framework 3.5). We don't actually need .NETStandard 2.0 in the benchmark source code. Such benchmarks can be executed with the help of our global tool. =) I guess, it will require a major refactoring and some hacks here and there, but it looks like a good approach.

@adamsitnik adamsitnik merged commit 5c519fb into dotnet:master Jan 13, 2019
@adamsitnik
Copy link
Member

@AndreyAkinshin @glennawatson thanks!

@AndreyAkinshin AndreyAkinshin added this to the v0.11.4 milestone Jan 13, 2019
@glennawatson glennawatson deleted the glennawatson-update-commandline branch January 13, 2019 18:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants