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

Remove Nerdbank.Streams dependency #592

Merged
merged 6 commits into from
Oct 24, 2019
Merged

Conversation

neuecc
Copy link
Member

@neuecc neuecc commented Oct 17, 2019

Currently only depends Sequence<T> but if depend it, adds many other dependency.
Working in #591 , I've encounted many annoying issues related to many managed dlls on Unity(and IL2CPP).
If removed Nerdbank.Streams, we can remove Microsoft.VisualStudio.Validation, Microsoft.VisualStudio.Threading, System.IO.Pipelines, too.
It will solve #566.

Copy and paste code is not a shame things.
For example, HTTP/2 HPack Encoder is copy and pasted share between Kestrel and HttpClient.
Piplined SslStream(SslDuplexPipe) is copy and pasted share between Kestrel and Orleans.
Compared to them, it is just one file.

@neuecc neuecc requested a review from AArnott October 17, 2019 11:05
Also replace the sync call with a true async implementation.
This fixes the remainder of the build breaks.
@AArnott
Copy link
Collaborator

AArnott commented Oct 21, 2019

The one downside to this change that I can see is that it makes Sequence<T> less discoverable for users of this library, and it can be useful in writing highly optimized code that reuses buffers.

@AArnott AArnott added this to the v2.0 milestone Oct 24, 2019
@neuecc neuecc merged commit 346488a into master Oct 24, 2019
@AArnott AArnott deleted the remove-nerdbank-depenedncy branch October 24, 2019 14:11
AArnott added a commit to AArnott/MessagePack-CSharp that referenced this pull request Nov 2, 2019
This spreads the intent of MessagePack-CSharp#592 to the source-distribution of MessagePack.
AArnott added a commit to AArnott/MessagePack-CSharp that referenced this pull request Nov 6, 2019
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.

None yet

2 participants