search

fsprojects / IfSharp

F# for Jupyter Notebooks
Other
442 stars 71 forks source link

Added reference Microsoft.NETCore.App version 2.2.3 #207

Closed rudihorn closed 5 years ago

rudihorn commented 5 years ago

This PR addresses issue #206.

cgravill commented 5 years ago

We mostly manage references using Paket at the moment. Is it possible to use Paket for this override as well?

rudihorn commented 5 years ago

I'll try to sort this out when I have some time

cgravill commented 5 years ago

I took at look into this and there's a suggestion we should use e.g.

<RuntimeFrameworkVersion>2.2.3</RuntimeFrameworkVersion>

https://docs.microsoft.com/en-us/dotnet/core/tools/csproj#recommendations

rather than directly referencing a (meta)package directly.

I added that and it seemed to build fine for me on Windows.

This might limit portability a bit. @dsyme you had an overlapping change in #209 any clarity you can provide on the right way to approach this?

dsyme commented 5 years ago

I'll definitely be taking a look at this. Constraining to a runtime version should not be needed for using TPs, so there is definitely something odd going on.

rudihorn commented 5 years ago

So I have a commit for the change as requested by @cgravill.

Admittedly @dsyme I haven't gone very in-depth into this problem, as to do that I would need to debug the FSharp.Compiler.Services library which was throwing somewhat unhelpful error messages. I did try to poke around in the code, but this wasn't straightforward enough without debugging.

cgravill commented 5 years ago

Thanks @rudihorn this avoids a warning too. In principal, I'm happy to merge this.

@dsyme it sounds like you're going to try to get to the bottom of this which would be even better.

rudihorn commented 5 years ago

Hmm for some reason I am unable to reproduce the error today and it seems to be working with just the Microsoft.NETCore.App=2.2.0 runtime reference. I'm wondering if this was me just thinking that the intellisense warning was not causing the command to be issued.

rudihorn commented 5 years ago

Yes I think it was just me just having a long day and seeing a red squiggly line with no output. I'll close this PR for now as I don't think it actually solves any issue.

cgravill commented 5 years ago

OK no problem, there are definitely things that need fixing with the .NET Core support even if this isn't quite the one.