-
Notifications
You must be signed in to change notification settings - Fork 982
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
Netstandard2 multi target #382
base: dev
Are you sure you want to change the base?
Conversation
update Microsoft.Extensions.Logging to 2.0.0 for netstandard2.0 target update dotnet sdk to 2.0.3 to support building netstandard2.0
Add netstandard 2.0 simply for logging does not add much value in my opinion. I would rather prefer a full migration to netstandard 2.0 and depreciate netstandard1.3. |
#if NETSTANDARD1_3 | ||
this.readByteCount = this.ReadFromInput(sslBuffer.Array, sslBuffer.Offset, sslBuffer.Count); | ||
#if NETSTANDARD1_3 || NETSTANDARD2_0 | ||
this.readByteCount = this.ReadFromInput(sslBuffer.Array, sslBuffer.Offset, sslBuffer.Count); | ||
// hack: this tricks SslStream's continuation to run synchronously instead of dispatching to TP. Remove once Begin/EndRead are available. |
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.
Begin/End Read/Write in already added in NETSTANDARD2_0, so maybe override them in this class(and the one in test)?
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.
@yyjdelete Exactly! There are things need to be cleaned up specifically for netstandard 2.0 before we officially support it. Supporting both 1.3 and 2.0 would make some part of code much harder to maintain. Plus, there are so many things we can take advantage of in netstandard 2.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.
I can attempt to help with that transition, but I'm not all that familiar with the internals of DotNetty or what specifically netstandard 2.0 added (other than a lot).
To me it seems like a larger project that should be broken out into pieces (perhaps starting with multi-target as the community works on the rest). It also seems that DotNetty is due for a release before that project gets started (would be really nice to get that http pr merged).
Great PR! Thanks. Any ETA? |
Adds netstandard2.0 as an additional target built.
Updates Microsoft.Extensions.Logging to 2.0.0 when using the netstandard2.0 target.
The motivation behind this is to fix a dependency conflict with MEL when using a different library with DotNetty that requires 2.0.0 and MEL 2.0.0 requires netstandard2.0.