search

googleapis / google-cloud-dotnet

Google Cloud Client Libraries for .NET
https://cloud.google.com/dotnet/docs/reference/
Apache License 2.0
939 stars 367 forks source link

Consider making Spanner libraries v2 only target netstandard2.0 #2988

Closed jskeet closed 5 years ago

jskeet commented 5 years ago

This is similar to #2959, but for Spanner.

Currently we have the following targeting:

Proposal: make all of these just target netstandard2.0.

This makes it simpler to reason about and simpler to test.

cc for input:

@FransBouma @tomerpeled @alyaros @jerarl @SurferJeffAtGoogle @chrisdunelm @amanda-tarafa

We may well want to change to make this the target for anything that hasn't already gone GA...

jskeet commented 5 years ago

PR #2989 is a speculative PR about what this would look like, but this is still just under consideration. I'm running tests now.

FransBouma commented 5 years ago

I use Google.Cloud.Spanner.Data (2.0 beta 8) in our designer on .NET 4.5.2 (as the minimum .NET version). Our customers should be able to run it on .net 4.5.2. Using netstandard2.0 for the spanner ADO.NET provider would make it fail I think, as .net 4.5.2 isn't 100% compatible with ns20 (AFAIK). If not, then I don't mind.

(edit) Spanner support in our product is partly funded by Google, it's crucial the designer (desktop app on .net 4.5.2+) works with the ADO.NET provider. We can't enforce .NET 4.7.2 onto our customers at this time.

jskeet commented 5 years ago

That would indeed mess things up - so I'm glad I asked for the input :)

I can make it target netstandard2.0 and net45 though, which still avoids the conditional code.

Thanks!

FransBouma commented 5 years ago

Oh that would be great! 😄 👍

tomerpeled commented 5 years ago

Should work on our side. Thanks!

jskeet commented 5 years ago

@FransBouma: We're currently planning a major version bump for all libraries, primarily due to Grpc.Core taking a major version bump, and as part of that we'd like to move from net45 to net461. That's the minimum version supported by Microsoft.Bcl.AsyncInterfaces, which we'd like to depend on. Are you still targeting net452 in your designer? I'd be very happy to discuss this in more detail by email (or a video conference) if you have the time. Please let me know how you'd like us to proceed.

FransBouma commented 5 years ago

I am, but I'm shipping the Spanner ADO.NET provider with the designer so it's not a big deal at the moment (the only requirement we have is that spanner works on .net framework by a direct target, not through ns20 as that would tie it to 4.7.2). We're busy with our new version, v5.7, which we can bump to a higher .NET version and with that ship a newer ADO.NET provider for spanner, e.g. the one which bumps the minimal .net version to 4.6.1.

So in the period till that release our users work with the ADO.NET provider we ship with the designer, and when we ship 5.7, we'll bump the minimal required version to 4.6.1 and ship the latest spanner ADO.NET provider with that. I think that would solve it. So I don't see any problems with you moving to 4.6.1.

The reason I posted my post above was that a requirement of 'netstandard 2.0 only' would be problematic so a dual targeting on .net framework would solve that and I think with a newer version being dual targeting ns2.0 and net461 would be OK.

Would that be sufficient for you to proceed as planned?

jskeet commented 5 years ago

@FransBouma: That's really good to know, thanks. Completely understand how netstandard2.0 would be a problem for you, and it's really great to get direct customer feedback like this - thank you!

Now I need to look at our stats to try to work out how many other customers might be disrupted by a move to net461... (It may well be "disruptive but still the best thing to do" - we'll see...)