Closed xepaul closed 9 years ago
This looks like some good stuff. Once the NuGet pieces have been rolled back, I'll take a deeper look. Also, I'm not very familiar with the MS Open Tech license. Is porting their code allowed? I assume so given RxJava; just want to check.
Also, thanks very much for the tests!
As mentioned earlier, I like what I see. However, I'm wondering if this should go into the core package. These look mostly like extensions to Rx, not F# wrappers, which is the intent of the library. Thoughts?
Possibly, I guess if this is purely F# wrappers for existing rx operators rather than F# support for rx in general. The Rx testing wrapper/port seems like a good addition as a separate assembly, and shows the need to have the relevant operators available with a IScheduler parameter?
@xepaul, I agree. I think it can go in this repo, but it should be a separate project in the solution.
@cloudRoutine what do you think? Another NuGet package?
I think adding it as another project in the solution is a good idea. I'd definitely like to see FSharp.Control.Reactive expand to cover a broader scope of Functional Reactive Programming in the future by incorporating libraries for Signal Graphs and Arrowized FRP.
As far as multiple NuGet packages are concerned, are you thinking of a structure where FSharp.Control.Reactive is (eventually) the overarching suite of libraries, with subset packages like FSharp.Control.Reactive.Core ( just the wrappers ) , FSharp.Control.Reactive.Testing, etc.?
Yep I think it makes sense to be another NuGet package.
I would love to get to this one, but it will take me a bit more time as it requires additional changes to the build.fsx file.
I've been having a ton of trouble getting this PR to work, and the overall structure of the repo has changed since you originally submitted the PR. Try branching off master and making your changes again in a new PR and we'll see if that one can work. Apologies for taking this long to address this, it probably would have been much easier to do this months ago.
Please remove the
.nuget
folders and files. We are now using Paket.