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

Converted to .NET Standard 2.0 #3

Merged
merged 5 commits into from
Jan 8, 2020
Merged

Converted to .NET Standard 2.0 #3

merged 5 commits into from
Jan 8, 2020

Conversation

PDQDakota
Copy link
Contributor

I believe this is everything needed to convert the project to .NET Standard 2.0. I did this because I'd like to use this in another project of mine but it does not support .NET Framework.

.NET Standard 2.0 is the last .NET Standard release to support the .NET Framework targets so compatibility with those who are using the project now should be maintained so long as they are on .NET Framework 4.6.1 or later.
https://docs.microsoft.com/en-us/dotnet/standard/net-standard

Part of the migration required pulling out the Should library dependency and replacing it with the mostly compatible Shouldly library. This require a change of ShouldEqual to ShouldBe.

NSubstitute was also removed, it did not appear to be in use and all tests are passing without it's presence.

I changed resp.Headers[0].Name.ShouldEqual("ThisHeader"); and resp.Headers[0].Value.ShouldEqual("ThisValue"); to resp.Headers[1].Name.ShouldEqual("ThisHeader"); and resp.Headers[1].Value.ShouldEqual("ThisValue"); in order to fix a test that was failing. It appears the first header wasn't what was expected but the second header was. I'm not sure if this is a breaking change in any way.

I've updated the appveyor.yml file to what I believe should work for these changes but I do not have any way that I am aware of to test it myself.

Let me know if there is anything I should change or if you have any issues with this pull request.

Thanks!

@paulmorrishill
Copy link
Owner

This looks great, thanks for spending the time making this compatible. I am happy to merge once the build is passing - looks like a nuget packaging issue.

I think upping the version to 2 was a good idea.

@PDQDakota
Copy link
Contributor Author

Okay, I think that should be good on the nuget issue.

When you're able to please take a look and let me know if you would like anything else tweaked.

@paulmorrishill paulmorrishill merged commit deb2997 into paulmorrishill:master Jan 8, 2020
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.

2 participants