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

Fix object-expr untested cases #17476

Merged
merged 10 commits into from
Aug 7, 2024
Merged

Conversation

edgarfgp
Copy link
Contributor

@edgarfgp edgarfgp commented Aug 1, 2024

Description

Continuation of #17388

Found some cases that weren't covered/tested before

Checklist

  • Test cases added

Copy link
Contributor

github-actions bot commented Aug 1, 2024

⚠️ Release notes required, but author opted out

Warning

Author opted out of release notes, check is disabled for this pull request.
cc @dotnet/fsharp-team-msft

@edgarfgp edgarfgp added the NO_RELEASE_NOTES Label for pull requests which signals, that user opted-out of providing release notes label Aug 1, 2024
@edgarfgp edgarfgp changed the title Fix object-expr untested case Fix object-expr untested cases Aug 1, 2024
@edgarfgp edgarfgp marked this pull request as ready for review August 2, 2024 13:02
@edgarfgp edgarfgp requested a review from a team as a code owner August 2, 2024 13:02
@edgarfgp
Copy link
Contributor Author

edgarfgp commented Aug 2, 2024

This is ready.

@edgarfgp edgarfgp closed this Aug 5, 2024
@edgarfgp edgarfgp reopened this Aug 5, 2024
@edgarfgp edgarfgp closed this Aug 5, 2024
@edgarfgp edgarfgp reopened this Aug 5, 2024
@psfinaki
Copy link
Member

psfinaki commented Aug 6, 2024

/azp run

Copy link

Azure Pipelines successfully started running 2 pipeline(s).

Copy link
Member

@KevinRansom KevinRansom left a comment

Choose a reason for hiding this comment

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

These test cases seem to verify that the compiler compiles the source code without producing errors, could we also verify that the built type has the interface. Adding a main with a cast and executing is probably sufficient.

I'm confident it does, but a test that confirms it might be useful.

@edgarfgp edgarfgp requested a review from KevinRansom August 6, 2024 18:02
@edgarfgp edgarfgp closed this Aug 7, 2024
@edgarfgp edgarfgp reopened this Aug 7, 2024
@edgarfgp
Copy link
Contributor Author

edgarfgp commented Aug 7, 2024

These test cases seem to verify that the compiler compiles the source code without producing errors, could we also verify that the built type has the interface. Adding a main with a cast and executing is probably sufficient.

I'm confident it does, but a test that confirms it might be useful.

Done 👍🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NO_RELEASE_NOTES Label for pull requests which signals, that user opted-out of providing release notes
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants