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

[okay to merge now] - move to .NET Standard exclusively #339

Merged
merged 38 commits into from
Oct 27, 2020

Conversation

cartermp
Copy link
Contributor

@cartermp cartermp commented Apr 10, 2020

  • Remove .NET Framework and Mono pieces of this repo
  • Adjust guidance so that we don't injure people with the "type provider that tries to also support older .NET Framework and .NET Standard and .NET Core and older Visual Studio" confuse-o-tron
  • Use paket consistently throughout the repo
  • Move TPDSDK to a proper project that can be published to NuGet
  • Remove more vestiges of non-.NET Standard assembly nonsense in testing and update all skipped tests appropriately
  • Update template
  • Solve the template generating some broke ass old paket.lock for some reason
  • Figure out why Provided Namespaces don't show in the BasicProvider unit test via project reference
  • Fix factoring of leaking implementation details for tests (namely the caching stuff)
  • Figure out stack overflow in tests
  • Do more targeted README updates to try and make it as simple as possible
  • Manually verify all examples

This simplifies the repo and how Type Providers build moving forward. The downside is that older .NET Framework and older F# consumers won't be able to use newer type providers build with the SDK in this state. That is also largely true today, though, as many type providers are updated and do not support these older environments.

@cartermp cartermp marked this pull request as ready for review April 10, 2020 23:25
@dsyme
Copy link
Contributor

dsyme commented Apr 14, 2020

This is great. We should probably create a branch marking the point just before we intergrate this in case people need to get the TPSDK for .NET Framework

@cartermp

This comment has been minimized.

@cartermp

This comment has been minimized.

@cartermp
Copy link
Contributor Author

someone was being very naughty and allowed zero tests to run in CI for a long time

@dsyme
Copy link
Contributor

dsyme commented Oct 26, 2020

This should now basically be ready (assuming it goes green). My one concern is the deletion of a chunk of important testing, I'll look into that now

The downside is that older .NET Framework and older F# consumers won't be able to use newer type providers build with the SDK in this state.

This should be clarified precisely in the README. For example

  • Will VS for Mac load a TPDTC built with the template?

  • Do we expect that the Mono compiler load a TPDTC built with the template?

  • Will VS2019 load a TPDTC built with the template, if so which is the earliest point version that will load it?

I don't really mind if the answers are "no" (though we should really know the answer for VS for Mac and Mono). But we should give some clarity since there are actual users with the above constraints who can't update to latest F# tooling.

Historically we've also often had enterprise users using back versions of Visual Studio. I so much wish there were ways of preventing even the referencing of a design time component based on certain constraints, but there isn't AFAIK.

I don't expect any problem from Ionide since it's updated so regularly.

@dsyme
Copy link
Contributor

dsyme commented Oct 26, 2020

Just to mention VS for Mac probably loads FSharp.Core 4.7.0.0 (package 4.7), see https://github.com/mono/monodevelop/blob/master/main/external/fsharpbinding/paket.lock#L14. It will load .NET Standard 2.0 components.

Mono tooling (fsc.exe etc.) looks like it will be using FSHarp.Core 4.5.0.0, see the commits in https://github.com/fsharp/fsharp. It will load .NET Standard 2.0 components.

So an FSharp.Core dependency of 4.5.2 makes sense to me after all. There's no reason TPDTC built with that dependency won't load, even if we're not actively testing those configurations.

Copy link
Contributor Author

@cartermp cartermp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Main thing is re-enabling the testing in templates and BasicProvider for the erasing providers. I commented them out just so that I could finish up other things but then never got back to completing this PR. I was going to request your help on this issue at the time.

@cartermp
Copy link
Contributor Author

Mono tooling (fsc.exe etc.) looks like it will be using FSHarp.Core 4.5.0.0

This will no longer be the case with mono/mono#20511 - both mono and VSMac will have the latest F# by VSMac 8.9 release, or a patch of 8.8

@dsyme
Copy link
Contributor

dsyme commented Oct 27, 2020

This will no longer be the case with mono/mono#20511 - both mono and VSMac will have the latest F# by VSMac 8.9 release, or a patch of 8.8

That is fantastic - is the VS for Mac PR public?

@dsyme
Copy link
Contributor

dsyme commented Oct 27, 2020

testing in BasicProvider

This is done, for BasicProvider, the relevant fix is here: https://github.com/cartermp/FSharp.TypeProviders.SDK/pull/1/files#diff-c540bf1e311c246e1a3b5149f3a17c045b769a48ae9b07e699fb98d62b4298a0R11

I'll look into the template now, it's possible those tests actually pass

@dsyme
Copy link
Contributor

dsyme commented Oct 27, 2020

OK the tests we want are all re-enabled and everything is green.

I think we can merge this. Next steps would be to

  1. push the pacakges
  2. update the README to be as simple as possible
  3. manually test the template

It's great to see how simple dependency management for TPDTC has become.

@dsyme
Copy link
Contributor

dsyme commented Oct 27, 2020

I checked the log and we really do seem to be instantiating the template and running tests (though output from running the tests is suppressed in the log it seems)

  Determining projects to restore...
  Restored D:\a\FSharp.TypeProviders.SDK\FSharp.TypeProviders.SDK\temp\tp250\src\tp250.DesignTime\tp250.DesignTime.fsproj (in 339 ms).
  Restored D:\a\FSharp.TypeProviders.SDK\FSharp.TypeProviders.SDK\temp\tp250\src\tp250.Runtime\tp250.Runtime.fsproj (in 15 ms).
  Restored D:\a\FSharp.TypeProviders.SDK\FSharp.TypeProviders.SDK\temp\tp250\tests\tp250.Tests\tp250.Tests.fsproj (in 1.99 sec).
  tp250.DesignTime -> D:\a\FSharp.TypeProviders.SDK\FSharp.TypeProviders.SDK\temp\tp250\src\tp250.DesignTime\bin\Debug\netstandard2.0\tp250.DesignTime.dll
  tp250.Runtime -> D:\a\FSharp.TypeProviders.SDK\FSharp.TypeProviders.SDK\temp\tp250\src\tp250.Runtime\bin\Debug\netstandard2.0\tp250.Runtime.dll
  tp250.Tests -> D:\a\FSharp.TypeProviders.SDK\FSharp.TypeProviders.SDK\temp\tp250\tests\tp250.Tests\bin\Debug\netcoreapp3.1\tp250.Tests.dll

Build succeeded.
    0 Warning(s)
    0 Error(s)

Time Elapsed 00:00:24.78
D:\a\FSharp.TypeProviders.SDK\FSharp.TypeProviders.SDK\temp\tp250> "C:\Users\runneradmin\AppData\Local\Microsoft\dotnet\dotnet.EXE"  test -c debug (In: false, Out: false, Err: false)
D:\a\FSharp.TypeProviders.SDK\FSharp.TypeProviders.SDK\temp> "C:\Users\runneradmin\AppData\Local\Microsoft\dotnet\dotnet.EXE"  new -u FSharp.TypeProviders.Templates (In: false, Out: false, Err: false)
Finished (TagStatus.Success) 'TestTemplatesNuGet' in 00:00:44.7358328
Starting target 'All'
Finished (TagStatus.Success) 'All' in 00:00:00.0001154

@dsyme
Copy link
Contributor

dsyme commented Oct 27, 2020

@cartermp I will merge this now. Thanks you so much for these simplifications, it's miraculous. Also thanks to @KevinRansom for getting IsFSharpDesignTimeProvider and friends into the SDK support, it makes a huge difference

@dsyme dsyme merged commit 8869600 into fsprojects:master Oct 27, 2020
@dsyme
Copy link
Contributor

dsyme commented Oct 27, 2020

I ❤️ merging PRs that say "DO NOT MERGE"

@cartermp
Copy link
Contributor Author

Lemme retro-actively adjust the title

@cartermp cartermp changed the title [Do not merge] - move to .NET Standard exclusively [okay to merge now] - move to .NET Standard exclusively Oct 27, 2020
@dsyme
Copy link
Contributor

dsyme commented Oct 27, 2020

Too late, it's in the git log for all eternity

Author: Phillip Carter <[email protected]>
Date:   Mon Oct 26 17:53:58 2020 -0700

    [Do not merge] - move to .NET Standard exclusively (#339)

@isaacabraham
Copy link

That's how we roll in F# land.

@schauerte
Copy link
Contributor

schauerte commented Oct 27, 2020

@dsyme

I checked the log and we really do seem to be instantiating the template and running tests (though output from running the tests is suppressed in the log it seems)

I think, this assumption may not be correct. Please take a look at #347.

@dsyme
Copy link
Contributor

dsyme commented Oct 27, 2020

@schauerte Thank you so much for double checking. That silent test failure is very disturbing!

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.

4 participants