search

michielpost / Q42.HueApi

C# helper library to talk to the Philips Hue bridge
MIT License
412 stars 114 forks source link

Build net462 by default #178

Closed leo60228 closed 5 years ago

leo60228 commented 5 years ago

Fixes #123.

michielpost commented 5 years ago

Hi,

Thanks for your PR. But the Q42.HueApi.Streaming project does not build for .Net45 It doesn't know about Task.CompletedTask and also _socket.ConnectAsync has different paremeters in .net45. Can you fix it so it builds successfully?

Thanks!

leo60228 commented 5 years ago

That's weird, it built for me. Looking at MSDN, it seems like those existed in .NET 4.5. Even then, isn't this library supposed to be compatible with .NET 4.5? For my uses, I only need .NET 4.7, I just chose 4.5 because of the README.

michielpost commented 5 years ago

The library targets .netstandard2.0, it's the way to go for modern .net development. If somebody needs additional frameworks and I can provide that without code changes, that's fine. But I won't put much effort in supporting legacy platforms. .NET 4.7.1 supports .netstandard2.0 packages, so including the net471 target is useless I think.

Thanks for pointing me to the readme, it contains a lot of outdated info. I'll update that.

leo60228 commented 5 years ago

.NET 4.7.1 supports netstandard2.0, yes, but it still pulls in several libraries from NuGet. For my usecase, it would be much more useful if I only needed to pull in Newtonsoft.Json and the 3 Q42.HueApi libraries.

leo60228 commented 5 years ago

Since for some reason .NET 4.5 built locally, even though it didn't on CI, I'm going to just bump the version until it works on CI, if that's okay with you.

leo60228 commented 5 years ago

Looks like it builds fine now. Though you might want to test it; not sure if I used SocketAsyncEventArgs correctly.

michielpost commented 5 years ago

Ok, so what if we target .net45 for the Q42.HueApi and ColorConverters project and .net471 for the Streaming project? That way .net45 is supported for the main project and if you want to use the Entertainment API you'll need at least .net471. And then we can leave the cleaner ConnectAsync as it is.

leo60228 commented 5 years ago

Okay, I'll do that.

michielpost commented 5 years ago

Thanks. Packages are published to NuGet with version 3.10.0