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

Update target frameworks #544

Closed
gitfool opened this issue Aug 16, 2020 · 11 comments
Closed

Update target frameworks #544

gitfool opened this issue Aug 16, 2020 · 11 comments

Comments

@gitfool
Copy link
Contributor

gitfool commented Aug 16, 2020

Following up from #514, Flurl really should seriously consider dropping unsupported target frameworks, especially given the upcoming bump in major version. What do you reckon, @tmenier?

@maxkatz6
Copy link

I don't see any good reason to drop any target framework.
In that PR you said only about reducing nupkg size, but is it really a problem? I mean, it is relatively small even now, and reducing size will not make any noticing difference for developers, who you this package.
From other hand, there are still applications, that can't upgrade their target framework to use netstandard2.0.

More meaningful reason to drop old targets is System.Text.Json (see
#517 ). But it needs some time to be more feature complete.

@tmenier
Copy link
Owner

tmenier commented Aug 16, 2020

@gitfool I'm not necessarily opposed to this if it's for the right reasons, and I've thought about it myself. Having seen the csproj file, what would you change specifically? (I'm not certain which frameworks are "unsupported" so I think it's easier to talk specific targets.)

@tmenier
Copy link
Owner

tmenier commented Aug 16, 2020

I'll give a brief rundown of where we're at currently with platform support. @kroniak has been a great help with these decisions in the past and I'm hoping he'll weigh in. To me, "nobody is likely using this platform anymore" should be a much greater factor than "unsupported".

So let's take a look at the relevant lines from Flurl.Http.csproj:

<TargetFrameworks>net45;net46;netstandard1.1;netstandard1.3;netstandard2.0;</TargetFrameworks>

That's the main focus of this discussion, obviously.

<TargetFrameworks Condition="'$(OS)' != 'Windows_NT'">netstandard1.1;netstandard1.3;netstandard2.0;</TargetFrameworks>

I'm not certain what the purpose of that is or if we still need it. @kroniak can you comment on this?

<PackageTargetFallback Condition="'$(TargetFramework)'=='netstandard1.1'">portable-net45+win8+wp8</PackageTargetFallback>

Gross. Maybe we're finally at a point where any remnants of PCL can be dumped? Although, since it's a fallback, it's not adding anything to the NuGet package. So one could argue it's harmless.

Here are the possible ways I could see going with this:

  1. Target netstandard2.0 only and greatly simplify everything. (Actually, according to this, adding net461 would probably be smart.)
  2. Drop only netstandard1.1 and that PCL awfulness. One perk is we could also drop the reference to PCLStorage, a somewhat obscure library that hasn't been maintained in over 5 years.
  3. Do nothing. No one's ever complained about the NuGet package size so why risk leaving anyone behind.

To me, option 2 seems like a reasonable compromise, if the risk of leaving anyone behind is extremely low. I could be persuaded to go in either of the other directions though, if there's strong opinions.

Good guidance here:

https://docs.microsoft.com/en-us/dotnet/standard/library-guidance/cross-platform-targeting

It's the first I've heard of MSBuild.Sdk.Extras. I wonder if that's worth looking into?

@tmenier tmenier added the 3.0 label Aug 16, 2020
@gitfool
Copy link
Contributor Author

gitfool commented Aug 17, 2020

Here are the possible ways I could see going with this:

Option 1 (.NET Standard 2.0 with .NET Framework 4.6.1) seems like the best option to me.

FWIW, in terms of unsupported frameworks, indeed it's a mess, requiring deciphering and correlating the following:

My takeaway from the above is that .NET Framework 4.5 / 4.5.1 are definitely unsupported, with newer .NET Frameworks being tied to the operating system. If you target .NET Framework 4.6.1 and above, you're only cutting loose Windows Vista SP2 / Windows Server 2008 SP2, leaving Windows 7 SP1 / Windows Server 2008 R2 SP1 and above (based on the supported operating systems). You'll also be able to cut loose .NET Standard 1.1 / 1.3 since .NET Framework 4.6.1 implements .NET Standard 2.0, albeit with issues, but such issues already exist with current targets anyway.

To me, "nobody is likely using this platform anymore" should be a much greater factor than "unsupported".

I agree but like everything "it depends". On the surface, "Option 1" seems like a good compromise, bearing in mind somebody will always be using unsupported platforms / targets, but are they really significant or just the vocal minority? Without metrics how will you even know the breakdown? Well, one way might be to just go ahead and make the breaking change and track how many people complain...

@mikehachen
Copy link

I would go for option 1...

tmenier added a commit that referenced this issue Sep 7, 2020
@tmenier
Copy link
Owner

tmenier commented Sep 7, 2020

I've gone with option 1 (the most "aggressive" changes). All feedback I've gotten indicates that's the preference, and I get some nice simplifications to the project. Win-win. :) Worst case, if it's discovered later that people need those older targets, adding them back won't be a breaking change.

If anyone cares to review the changes to the csproj files (especially @kroniak), here's the commit: 63f61cb

@kroniak
Copy link
Collaborator

kroniak commented Sep 8, 2020

this is a great step. I'm with you.

tmenier added a commit that referenced this issue Sep 8, 2020
@tmenier
Copy link
Owner

tmenier commented Oct 1, 2020

Released: https://www.nuget.org/packages/Flurl.Http/3.0.0-pre5

@tmenier
Copy link
Owner

tmenier commented May 23, 2022

I'm curious if any of you who participated in this conversation a couple years ago has any new thoughts for a .NET 6 / Flurl 4 world. It looks like the advice here hasn't changed much (if it all) - still netstandard2.0 for broadest coverage and a dash of net461 for good measure. Anyone disagree about that continuing to be the right combination going forward?

Somewhat less important (but still important), I'd like to update the target frameworks for the test project. Every new target adds to the test run time so I don't want to go too crazy, but thinking of going with: net6.0;netcoreapp3.1;net461.

Thoughts?

@tmenier tmenier reopened this May 23, 2022
@michaelhachen
Copy link

netstandard2.0 and net461 works for me...

@gitfool
Copy link
Contributor Author

gitfool commented May 27, 2022

I would add a net6.0 target since it's an lts release and replaces netstandardx.x going forward.

tmenier added a commit that referenced this issue Jun 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Released
Development

No branches or pull requests

6 participants