search

braintree / braintree_dotnet

Braintree .NET library
https://developer.paypal.com/braintree/docs/start/overview
MIT License
136 stars 73 forks source link

Cross target .NETCOREAPP3.1 to remove transient dependencies #110

Closed iamcarbon closed 2 years ago

iamcarbon commented 3 years ago

Summary

Checklist

This PR cross targets .NETCOREAPP3.1 and conditionally upgrades Newtonsoft.Json to avoid bringing in any additional dependencies or facades on modern runtimes.

hollabaq86 commented 3 years ago

👋 @iamcarbon thanks for the PR. Can you elaborate on your reason for this PR?

Introducing an additional target framework increases the complexity of managing our SDK, so I want to make sure we're being considerate of .NET folks who consume our SDK while managing our maintenance workload.

iamcarbon commented 3 years ago

Hi @hollabaq86. The System.Xml.XPath.XmlDocument dependency brings in 10+ direct dependencies and even more transient dependencies when run on .NETCOREAPP3.1+.

These libraries were temporary artifacts before the re-unification of .NET cumulating in the .NET 5.0 release and have not been updated since 2016. Eliminating these artifacts speeds up build times, reduces deployment sizes, and eliminates the chances of a future library conflict.

It also helps assure that the library is considering and testing the runtimes that the .NETSTANDARD libraries abstract. .NETCOREAPP3.1 is the latest LTS release for .NET and promoted as the best choice for developing any new .NET applications needing LTS.

hollabaq86 commented 3 years ago

👋 @iamcarbon I spun up a Core 2.1 container with these changes to make sure we're not breaking compatibility with folks still on that version, and I'm seeing the following build errors:

/usr/share/dotnet/sdk/2.1.814/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.TargetFrameworkInference.targets(137,5): error NETSDK1045: The current .NET SDK does not support targeting .NET Core 3.1.  Either target .NET Core 2.1 or lower, or use a version of the .NET SDK that supports .NET Core 3.1. [/home/admin/bt/braintree-dotnet/src/Braintree/Braintree.csproj]

A quick way to reproduce this issue would be to pull the code from our old dockerfile-core2 file, update for Core 2.1, and modify the Makefile commands to build this container on your machine.

I'm a little stumped on potential workarounds for this issue... if we're unable to find a solution that doesn't break existing functionality for Core 2.1 users we'll need to pass on this PR until we can drop Core 2.1 support in Aug 2021.

iamcarbon commented 3 years ago

@hollabaq86 Bump now that that .NETCOREAPP2.1 is EOL.

cgdibble commented 2 years ago

Hey @iamcarbon, thanks for the bump! You are right in that 2.1 is EOL. We definitely have this work in the pipeline, but it will be coming with a broader set of work for the next major version of the SDK.

smendoza88 commented 2 years ago

@cgdibble bump 3.1 EOL is Dec 3, 2022