jack-pappas / ExtCore

An extended core library for F#.
Apache License 2.0
178 stars 32 forks source link

WIP Move to .NET Standard 2.0 #27

Closed vasily-kirichenko closed 6 years ago

vasily-kirichenko commented 6 years ago

Open issues:

image

image

This is an interesting issue: Seq module is defined twice in ExtCore.Collections namespace, once with ModuleSuffix attribute and once - without it:

/// Additional functional operators on sequences.
[<RequireQualifiedAccess; CompilationRepresentation(CompilationRepresentationFlags.ModuleSuffix)>]
module ExtCore.Collections.Seq
namespace ExtCore.Collections
...
module Seq =
    /// <summary>
    /// Converts asynchronous sequence to a synchronous blocking sequence.
    /// The elements of the asynchronous sequence are consumed lazily.
    /// </summary>
    /// <param name="input"></param>
    /// <returns></returns>
    [<CompiledName("FromAsyncSeq")>]
    let ofAsyncSeq (input : AsyncSeq<'T>) : seq<'T> =
        AsyncSeq.toSeq input

I've commented out ofAsyncSeq and it's fixed the compilation. I suspect it's a regression in F# 4.1

/cc @dsyme

Tests.ExtCore.Control.Collections.Async+Seq+Parallel.batch

  Expected is <System.Int32[10]>, actual is <System.Int32[24]>
  Values differ at index [3]
  Expected: 16
  But was:  4

   at Tests.ExtCore.Control.Collections.Async.Seq.Parallel.batch() in E:\github\ExtCore\ExtCore.Tests\ControlCollections.Async.fs:line 1092

I'd like to hear what do you guys thought of all this. Is this the right direction? Have I done everything properly? Thanks!

jack-pappas commented 6 years ago

This is great! Thanks Vasily! Let me take a look over it today and tomorrow.

The one test that's failing is a known bug in the implementation of the Async,Seq.Parallel.batch function -- see #15.

/cc @7sharp9 @jindraivanek

vasily-kirichenko commented 6 years ago

About the issue with duplicated Seq module, see https://github.com/Microsoft/visualfsharp/issues/3565

vasily-kirichenko commented 6 years ago

Apparently NUnit Core test adapter does not support 2.0 https://github.com/nunit/dotnet-test-nunit/issues/122

vasily-kirichenko commented 6 years ago

AppVeyor tries to build it with msbuild 12 and fails.

vasily-kirichenko commented 6 years ago

It installs .net core 2.0, builds the solution and run the tests on Core. All on Windows. I don't know what to do next.

jack-pappas commented 6 years ago

Thanks Vasily! I've merged your changes. I think I need to make a few more tweaks to the build process, but I'll work on it today. (A few things in ExtCore use F# features restricted to FSharp.Core and require fsc-proto to build. It's possible to build ExtCore with a regular F# compiler but those things are excluded with a preprocessor macro.)

vasily-kirichenko commented 6 years ago

Thanks!