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

Add support for IAsyncEnumerable #175 #176

Merged
merged 5 commits into from
Oct 25, 2022
Merged

Add support for IAsyncEnumerable #175 #176

merged 5 commits into from
Oct 25, 2022

Conversation

cmxl
Copy link
Contributor

@cmxl cmxl commented Apr 3, 2022

Solves #175

IAsyncEnumerable is only available since netstandard2.1.
So I made the EndpointBase classes partial, added a new partial class file and removed it from compilation when the targetframework is not netstandard2.1

What I was not sure of, is how to add test for this feature.

@ardalis
Copy link
Owner

ardalis commented Apr 18, 2022

Can you add any tests or something in the sample demonstrating its usage?

@cmxl
Copy link
Contributor Author

cmxl commented Apr 19, 2022

I just added tests and a sample endpoint.
I had to add a direct project reference though, due to automapper failing to load the correct assembly when referencing a multitargeted project. That's also why I needed to add <SetTargetFramework>TargetFramework=netstandard2.1</SetTargetFramework> to the project reference in csproj files.
Otherwise the netstandard2.0 version would have been referenced by default.

@@ -19,7 +19,10 @@
</ItemGroup>

<ItemGroup>
<ProjectReference Include="..\SampleEndpointApp\SampleEndpointApp.csproj" />
<ProjectReference Include="..\..\src\Ardalis.ApiEndpoints\Ardalis.ApiEndpoints.csproj">
<SetTargetFramework>TargetFramework=netstandard2.1</SetTargetFramework>
Copy link
Contributor

@maxkoshevoi maxkoshevoi Apr 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this be an issue for NuGet consumers?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it makes sense to migrate whole package to netstandard2.1. It's only intended to be used in ASP.NET Core anyway

Copy link
Contributor Author

@cmxl cmxl Apr 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this be an issue for NuGet consumers?

I remember reading something about it, but did not have time to try.

Maybe it makes sense to migrate whole package to netstandard2.1. It's only intended to be used in ASP.NET Core anyway

I would personally like to upgrade to netstandard2.1.
The compatibility table is here:
https://docs.microsoft.com/en-us/dotnet/standard/net-standard?tabs=net-standard-2-1#select-net-standard-version

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ardalis How about updating to netstandard2.1?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ardalis any thoughts about upgrading to netstandard2.1?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes that's fine with me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#180 has bumped the target framework to netcore3.1, so I removed the netstandard2.1 stuff.
code is so much cleaner now :)

@ardalis ardalis merged commit bd1b256 into ardalis:main Oct 25, 2022
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.

3 participants