Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated target to netcoreapp3.0 #1909

Closed
wants to merge 1 commit into from
Closed

Conversation

ncave
Copy link
Collaborator

@ncave ncave commented Sep 24, 2019

No description provided.

@ncave ncave force-pushed the master branch 3 times, most recently from f26b900 to 04a8ad4 Compare September 24, 2019 04:58
@alfonsogarciacaro
Copy link
Member

Thanks @ncave! Have you noticed any performance improvements when upgrading to netcore 3? I was considering to postpone the upgrade until Fable 3, but maybe it's better to just do it with a minor release. I'm concerned that some users may not have netcore 3 installed yet though so latest version wouldn't run in their machines (we could add a version check before running Fable.Cli.dll to prevent this though).

@ncave
Copy link
Collaborator Author

ncave commented Sep 24, 2019

@alfonsogarciacaro netcore 3.0 works great out of the box, I don't see a lot of performance improvement for Fable, but that's probably because F# and FCS are not using more of netcore 3.0 features (yet). It's up to you, but this is an official release and is the version that people get by default when they install .NET Core. Or you can wait until 3.1 (LTS) is released, but I don't think much it's going to change, so we might as well switch now. In any case there is no point of doing 2.2 anymore as it is going away.

@ncave
Copy link
Collaborator Author

ncave commented Sep 24, 2019

@alfonsogarciacaro Travis CI fail is possibly related to #1910, although it works fine for me on WSL/Ubuntu 18.04.
Perhaps removing this line will fix it?

@alfonsogarciacaro
Copy link
Member

Hmm, it seems the RuntimeFrameworkVersion is already removed in this PR but it's still failing in Travis. #1910 seems to be about running the current Fable version in a Linux machine with dotnet SDK 3 installed. Looks a bit like #1807.

IIRC I originally added RuntimeFrameworkVersion to prevent MSBuild from implicitly selecting the runtime version of the dotnet SDK in my machine which caused problems, see #963.

I hate when they release a new netcore version and then everything starts to fail... 😬

@forki
Copy link
Collaborator

forki commented Sep 25, 2019

@alfonsogarciacaro I don't think it's required to move to dotnet core 3 with the project in minor. But the roll forward thing (described in #1910) would be really appreciated. Right now we can't run fable on sdk 3

@ncave ncave force-pushed the master branch 5 times, most recently from c89c8cb to 5e194ed Compare September 25, 2019 15:40
@ncave
Copy link
Collaborator Author

ncave commented Sep 25, 2019

@alfonsogarciacaro We may not be completely out of the woods yet, seeing that in Travis fable-compiler v2.4.2 works fine when targeting netcoreapp2.1 on sdk: 3.0.100, but not when targeting netcoreapp3.0 on sdk: 3.0.100.

@ncave ncave changed the title Updated dotnet version to 3.0 Updated target to netcoreapp3.0 Sep 25, 2019
@alfonsogarciacaro
Copy link
Member

Well, given that the app is working with the netcoreapp2.1 target now even if the user only has netcore 3 installed in their computer, I guess there's no rush to upgrade to netcoreapp3. We can keep the PR open and wait until either a new dotnet SDK or something in Travis fixes the issue. Or we can also wait until netcoreapp3.1 to get LTS support.

@ncave
Copy link
Collaborator Author

ncave commented Sep 25, 2019

@alfonsogarciacaro My point was, what if somebody is trying to compile with Fable a project that targets netcoreapp3.0 on sdk 3.0, perhaps there is still an issue (not on Windows, but still).
But yes, this PR can wait until 3.1, I'll close it.

@ncave ncave closed this Sep 25, 2019
@forki
Copy link
Collaborator

forki commented Sep 25, 2019

@ncave what would that do? Would fable treat them differently? IIRC I always have netstandard as target framework for fable. Is that wrong?

@ncave
Copy link
Collaborator Author

ncave commented Sep 25, 2019

@forki No, nothing like that, I was just saying that the Travis logs in this PR may be indicative of a potential issue when compiling projects targeting netcoreapp3.0 on sdk 3.0.
Somebody should just try that on a non-windows machine or VM.

@alfonsogarciacaro
Copy link
Member

alfonsogarciacaro commented Sep 25, 2019

Yes, I usually recommend to target netstandard2.0 (I think this also avoids issues with FSharp.Core references) as Fable projects don't need to be actual dotnet apps. But I guess @ncave is thinking in Fable projects that are intended to become node apps and use the EntryPointAttribute (like fable-compiler-js itself). In those cases you do need to target netcoreapp.

@forki
Copy link
Collaborator

forki commented Sep 26, 2019 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants