braintree / braintree_dotnet

Braintree .NET library

https://developer.paypal.com/braintree/docs/start/overview

MIT License

136 stars 73 forks source link

Cross target NETSTANDARD2.0 #81

Closed gitvinit closed 5 years ago

gitvinit commented 5 years ago

This update will help cross target NETSTANDARD2.0

Summary

Checklist

[x] Added changelog entry
[x ] Ran unit tests (See README for instructions)

fixes #78

iamcarbon commented 5 years ago

Can we also update Microsoft.AspNetCore.Http.Abstractions to 2.2?

gitvinit commented 5 years ago

@iamcarbon thanks for reviewing the change and for the package update suggestion Microsoft.AspNetCore.Http.Abstractions 2.2.0 is not compatible with netstandard1.3 should I separate the package references by netstandard version? `

<PackageReference Include="System.Xml.XPath.XmlDocument" Version="4.3.0" />

and netstandard1.3 will have

<PackageReference Include="System.Xml.XPath.XmlDocument" Version="4.0.1" />

iamcarbon commented 5 years ago

Curious if we can just remove the Microsoft.AspNetCore.Http.Abstractions dependency all together. I just did this locally and all tests continue to pass. It doesn't look like anything is using it.

@jackellenberger -- do you have ideas on why this dependency was introduced or know of any unintended consequences if we remove it? If we keep it, it's going to introduce the possibility for binding redirect problems.

jackellenberger commented 5 years ago

@iamcarbon The Abstractions dependency was added back when we started supporting .net core 1.x. I'm not sure if anything has changed regarding it being a requirement, but we can take it out and see if anything complains. I don't see anything asking for it explicitly.

gitvinit commented 5 years ago

@jackellenberger thanks for approving the changes updated changelog and reverted changes for .sln file @iamcarbon thanks for your help throughout the process