-
Notifications
You must be signed in to change notification settings - Fork 768
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
Multi-target .NET 5 #987
Multi-target .NET 5 #987
Conversation
ee4ef37
to
6ec8531
Compare
Interop tests are failing because of grpc/grpc#23523 |
e6710f2
to
a3295c1
Compare
Everything is passing apart from interop tests @jtattermusch When you have time can you help with grpc/grpc#23523. Need to update grpc/grpc so changing the framework doesn't change the path and break interop tests. |
a3295c1
to
bd424f9
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.
LGTM for adding net5 targets, @JunTaoLuo can you please review the rest of the changes in detail?
global.json
Outdated
@@ -1,5 +1,5 @@ | |||
{ | |||
"sdk": { | |||
"version": "3.1.300" | |||
"version": "5.0.100-rc.1.20367.2" |
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.
should we explicitly set rollForward ?
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 don't know. I have used that setting.
Is it required? I haven't had trouble with installing newer versions yet.
df49146
to
78b9dcc
Compare
9ad48fb
to
0a17f36
Compare
0a17f36
to
60ad9ca
Compare
@JunTaoLuo This is ready to review |
|
||
<Project Sdk="Microsoft.NET.Sdk.Web"> | ||
<!-- | ||
TODO(JamesNK): SDK temporarily changed from Microsoft.NET.Sdk.Worker to Microsoft.NET.Sdk.Web |
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.
File an issue so we don't forget to do this in the future?
src/Grpc.Net.Client/GrpcChannel.cs
Outdated
using Grpc.Core; | ||
using Grpc.Net.Client.Internal; | ||
using Grpc.Net.Compression; | ||
using Microsoft.Extensions.Logging; | ||
using Microsoft.Extensions.Logging.Abstractions; | ||
using System; |
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.
Why?
src/Grpc.AspNetCore.Server/Internal/CallHandlers/ServerCallHandlerBase.cs
Show resolved
Hide resolved
4d4d518
to
cbb059a
Compare
Testing multi-target packages on .NET 5
Soon the client and server will start using .NET 5 specific APIs, e.g. configuring the client to create a connections when there are more than 100 concurrent requests.