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

Adding support for .NET Core #36

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

aclemmensen
Copy link

This branch attempts at a conversion of the library to .NET Core with project.json/xproj files. I'm motivated to do this because another library I use (Disque.Net) uses this library, and converting this to .NET Core is a blocker in getting Disque.Net over.

The primary issues I've had in the conversion was the dependency on System.Runtime.Serialization, but as it turned out the only real dependency in that namespace was FormatterConverter, and all it contains is a bunch of calls to System.Convert which, thankfully, is available on Core.

I updated the test project to be a Core project as well, and all the tests are passing.

The only contract level change I had to make that may affect consumers is that IRedisSocket now returns a Task<Stream> instead of a Stream in the GetStream method. This is to accommodate SslStream which now only exposes an async method for authenticating.

The other potentially troublesome change I made is that the relationship between OnConnected and OnAsyncConnected on RedisConnector has been inverted to account for the change described above.

I also had to disable strong-name signing of the assemblies to get the tests to run locally.

Copy link

@RolandColored RolandColored left a comment

Choose a reason for hiding this comment

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

We are already using this functionality and it's running fine!

@tonyqus
Copy link

tonyqus commented Oct 17, 2020

nice work

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