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 58 forks source link

Correct Observable.filteri bug and optimise Observable.mapi #72

Closed anton-pt closed 8 years ago

anton-pt commented 8 years ago

Corrected the definition of Observable.filteri which previously only took a predicate of type 'T -> bool and simplified the definition of Observable.mapi, also improving performance.

anton-pt commented 8 years ago

Hmmm, so I was thinking of bringing in FsCheck to do the tests but to do so I need to upgrade the project to .NET 4.5. Any objections?

cloudRoutine commented 8 years ago

by all means please do

panesofglass commented 8 years ago

:+1:

anton-pt commented 8 years ago

Anything else needed before this is ready to be merged in? Sorry: I don't have a lot of experience contributing to OSS and doing unit testing....

smoothdeveloper commented 8 years ago

FSharp Core 4.0 works with Framework 4.0.

I'm not using this library at this moment, but I do maintain software that needs to ship for .NET 4.0.

If you are changing the framework just because of most recent FSharp.Core that might not be the right decision.

Maybe tests can be put in a separate project which targets 4.5.2?

anton-pt commented 8 years ago

@smoothdeveloper valid point: there should be no need for the main project to change, only the test project. I think it was FsCheck that required 4.5. I will roll back the main project to 4.0 when I get a chance.

cloudRoutine commented 8 years ago

@anton-pt once you fix the framework settings this is good to merge

anton-pt commented 8 years ago

I made the change, however I ran into a problem. I believe that it's the same issue discussed here. It's a bit over my head to fix but, in that discussion, @forki mentions that there is a way to do so?