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

Add FSharpType.IsMeasureType #14978

Merged
merged 3 commits into from
Apr 5, 2023
Merged

Add FSharpType.IsMeasureType #14978

merged 3 commits into from
Apr 5, 2023

Conversation

alfonsogarciacaro
Copy link
Contributor

It looks there are some measure types that don't have type definition, like Measure.Var or Measure.RationalPower. This makes it impossible to know if the type is a measure from the FCS public API, so this PR exposes a new member for that.

Copy link
Member

@T-Gro T-Gro left a comment

Choose a reason for hiding this comment

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

Approved, just SurfaceArea for FCS needs to be updated.

@alfonsogarciacaro
Copy link
Contributor Author

Thanks @T-Gro! How the SurfaceArea is updated?

@psfinaki
Copy link
Member

@alfonsogarciacaro thanks Alfonso :)
You can see an example of baseline update in this recent PR.

See also this PR, the contributor was confused just like you are :) Last commits in that PRs do the right thing though.

@T-Gro
Copy link
Member

T-Gro commented Apr 3, 2023

Thanks @T-Gro! How the SurfaceArea is updated?

Either manually in the respective surfacearea text file (a text description of the public API).
Or, an environment variable can be set, which sets the surface area when the test is run:

    $env:TEST_UPDATE_BSL=1 
    dotnet test tests/FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.Tests.fsproj --filter "SurfaceAreaTest"
    dotnet test tests/FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.Tests.fsproj --filter "SurfaceAreaTest" -c Release

@psfinaki psfinaki merged commit 0825e8c into dotnet:main Apr 5, 2023
@alfonsogarciacaro
Copy link
Contributor Author

Thanks for merging and sorry for not updating the surface area myself 😅

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

Successfully merging this pull request may close these issues.

4 participants