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

Intent to drop .NET Core App 2.1 as target framework #566

Closed
nozzlegear opened this issue Aug 22, 2020 · 11 comments
Closed

Intent to drop .NET Core App 2.1 as target framework #566

nozzlegear opened this issue Aug 22, 2020 · 11 comments
Labels

Comments

@nozzlegear
Copy link
Owner

Related to #438, I intend to drop netcoreapp2.1 as a target framework. Note that the package still supports netstandard 1.4, which is the reason I'm going to drop the core app as a target. Somebody can correct me if I'm wrong here, but there's no need to target netcoreappX when we're already targeting netstandard, as the netstandard package will continue working in a .NET Core app without any changes. I think I was just a little confused about the new .NET versioning/framework naming scheme when it was added as a target.

@nozzlegear nozzlegear pinned this issue Aug 22, 2020
@ChaoticIke
Copy link
Contributor

Our 2.1 applications have some dependencies on some netstandard1.x packages. So it shouldn't be a problem.

Microsoft provides a netstandard compatibility table here:
Interactive Table
image

@clement911
Copy link
Collaborator

I'm trying to add helper to drastically simplify paging with IAsyncEnumerable<T>.
Unfortunately, this requires .NET Core 3.0 or .NET Standard 2.1 so I can't put it in until we drop some older targets...

@ChaoticIke
Copy link
Contributor

@clement911 Have you looked at Microsoft.Bcl.AsyncInterfaces it adds the support for IAsyncEnumerable<T> for net standard 2.0 / framework 4.6.1. Since some of the usability features are in c#8 the syntax could be different.

@clement911
Copy link
Collaborator

I'm not too sure. Between the TFMs and the lang version it gets pretty confusing.
Does it allow to do await foreach(...)?

@ChaoticIke
Copy link
Contributor

That is part of C#8 so you would need to use the core 3.0 SDK to build the code. You can still target a lower framework

Found an example here:
IAsyncEnumerable Is Your Friend, Even In .NET Core 2.x

@clement911
Copy link
Collaborator

Looks like this package requires .NETFramework 4.6.1 minimum but ShopifySharp still targets .NET45.
@nozzlegear , where do we stand on dropping support NET45 and NET CORE 2.1.
Can we not just target .Net standard 2? See https://docs.microsoft.com/en-us/dotnet/standard/net-standard#net-implementation-support

@clement911
Copy link
Collaborator

@ChaoticIke while it might be possible to implement right now with a combination of #ifdef and conditional references, I'm not too keen on adding such complexity. I'd rather wait until we drop old targets and have a simpler implementation.

@clement911
Copy link
Collaborator

FYI, I ended writing a helper function in my own code.

        public static async IAsyncEnumerable<ListResult<T>> ListPagesAsync<T>(Func<ListFilter<T>, Task<ListResult<T>>> getPageAsync, ListFilter<T> filter)
        {
            while (true)
            {
                var res = await getPageAsync(filter);

                yield return res;

                if (!res.HasNextPage)
                    break;

                filter = res.GetNextPageFilter();
            }
        }

@nozzlegear
Copy link
Owner Author

@clement911 My plan is to drop the framework support in 6.0 -- not clear on when that release will come/be needed. .NET Core can be dropped at any point since it's not a breaking change for anyone, I might even do it in the next release.

@kdcllc
Copy link

kdcllc commented Feb 13, 2021

@nozzlegear why can't the library simply target netstandard? It should work in .NET Framework and DotNetcore. From my quick scanning of the code I do not see the issue with supporting only netstandard. Thoughts?

@nozzlegear
Copy link
Owner Author

netcoreapp2.1 support dropped in version 5.12.0. The package still targets netstandard1.4 which is compatible with .NET Core apps.

@nozzlegear nozzlegear unpinned this issue Nov 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants