search

fsprojects / FSharp.Control.Reactive

Extensions and wrappers for using Reactive Extensions (Rx) with F#.
http://fsprojects.github.io/FSharp.Control.Reactive
Other
284 stars 60 forks source link

Lower requires FSharp.Core and clean out TFMs #161

Closed cartermp closed 3 years ago

cartermp commented 3 years ago

Fixes https://github.com/fsprojects/FSharp.Control.Reactive/issues/160

Also clears out what appears to be redundant TFM usage (it really should not be needed, since there are no conditional defines for each target). and some stuff in the travis build.

cartermp commented 3 years ago

Let's see if my nonsense in the travis file was right or not.

cartermp commented 3 years ago

Looks like it was!

deviousasti commented 3 years ago

Hey Philip,

The multiple targets frameworks - IIRC, using this in netstandard 2.0 adds a reference to Tasks Extensions which isn't necessary in net5.0.

cartermp commented 3 years ago

According to the docs that's not the case, but I may be looking at the wrong thing: https://docs.microsoft.com/en-us/dotnet/api/system.threading.tasks.taskextensions?view=netstandard-2.0

In either case, I'm definitely not married to removing the multiple TFMs. That's just a quick thing I saw and thought wasn't necessary.

deviousasti commented 3 years ago

So, System.Reactive also ships versions for netstandard2.0 and net5.0, probably for the same reasons. Maybe we should align it with their TFMs.

image

deviousasti commented 3 years ago

Merged. Thanks @cartermp!

panesofglass commented 3 years ago

@deviousasti if you don't mind tagging this, it should automatically deploy the updated packages. An update in the RELEASE_NOTES would be nice, too. Thanks for reviewing and merging!

deviousasti commented 3 years ago

@panesofglass Done. Updated the release notes, and tagged 5.0.1. Thanks!

deviousasti commented 3 years ago

@panesofglass While FSharp.Core was updated in paket.dependencies to >= 4.7.2, we needed to change it in paket.lock for it to take effect.

cartermp commented 3 years ago

Hmmm. I did a paket install locally but maybe that wasn't picked up in my PR?

deviousasti commented 3 years ago

From paket docs:

Unlike paket update, paket install will only look for new versions of dependencies that have been modified in paket.dependencies and use the version from paket.lock for all other dependencies.

I think it means that since the lock version was already at 5.0.1, it won't downgrade.

deviousasti commented 3 years ago

Fixed in #162.