-
Notifications
You must be signed in to change notification settings - Fork 814
Update solution to .NET 7 #1803
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
Conversation
21fb3f6
to
3ae6a3c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a few comments. I'd also like @captainsafia to review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the inclusion of src/Grpc.AspNetCore/lib/net7.0/_._
intentional?
Looks good other than some non-blocking comments inline.
<PropertyGroup> | ||
<TargetFrameworks Condition="'$(LatestFramework)'!='true'">net6.0</TargetFrameworks> | ||
<TargetFrameworks Condition="'$(LatestFramework)'=='true'">net6.0</TargetFrameworks> | ||
<TargetFrameworks Condition="'$(LatestFramework)'!='true'">net7.0</TargetFrameworks> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not directly related to this change, but why are two different conditions needed here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interop tests are also run from grpc/grpc repo, and may specify a different framework.
</ItemGroup> | ||
|
||
<ItemGroup Condition="$([MSBuild]::VersionGreaterThanOrEquals($(TargetFrameworkVersion), 5.0))"> | ||
<ItemGroup Condition="'$(TargetFramework)' == 'net5.0' OR '$(TargetFramework)' == 'net6.0' OR '$(TargetFramework)' == 'net7.0'"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason we're no longer VersionGreaterThanOrEquals
? It's a bit more future-proof.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't get it to work after adding net7.0 target 🤷
Yes. Grpc.AspNetCore is a metapackage and doesn't have a DLL itself. Adding this file keeps the build happy. |
No description provided.