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

Refactor expression checking #17456

Merged
merged 15 commits into from
Jul 30, 2024

Conversation

vzarytovskii
Copy link
Member

  1. Split CEs checking into its own file
  2. Split seq checking into its own file
  3. Split array/list expressions checking into its own file
  4. Put common operations into *Ops.fs file.
  5. Refactor CE checking - separate nested function into top-level ones, clean up 2k+ lines main checking function.

It's a bit painful to work with now, so I decided to split it up and re-organize.

Copy link
Contributor

github-actions bot commented Jul 29, 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

@vzarytovskii vzarytovskii marked this pull request as ready for review July 29, 2024 13:23
@vzarytovskii vzarytovskii requested a review from a team as a code owner July 29, 2024 13:23
@vzarytovskii
Copy link
Member Author

This is ready once passing. After that I will be refactoring CE checking, so it's easier to argue about its perf.

@vzarytovskii vzarytovskii added the NO_RELEASE_NOTES Label for pull requests which signals, that user opted-out of providing release notes label Jul 29, 2024
@vzarytovskii
Copy link
Member Author

vzarytovskii commented Jul 29, 2024

I intend to merge it once tests are passing, so I can bring next PR of factoring out a bunch of helper functions out of TcComputationExpression function (it's now like 2k lines). Let me know if it shouldn't be done.

@psfinaki psfinaki changed the title [WIP] Refactor expression checking Refactor expression checking Jul 30, 2024
@psfinaki psfinaki enabled auto-merge (squash) July 30, 2024 11:56
@psfinaki psfinaki merged commit c68a301 into dotnet:main Jul 30, 2024
30 checks passed
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