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

Migrate Extrude AST mod from XState action to actor #5146

Merged
merged 8 commits into from
Jan 31, 2025

Conversation

franknoirot
Copy link
Collaborator

Part of our drive to clean up incorrect usage of XState to eliminate race conditions in the app. Actions never await or are asynchronous, so the async callback within this action was never properly handled.

This change was also made as a part of #5045 but I've broken it out here in case this can get in sooner and @adamchalmers can benefit from it for his kwargs work.

Copy link

vercel bot commented Jan 23, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
modeling-app ✅ Ready (Inspect) Visit Preview Jan 31, 2025 4:22pm

Copy link

qa-wolf bot commented Jan 23, 2025

QA Wolf here! As you write new code it's important that your test coverage is keeping up.
Click here to request test coverage for this PR!

Copy link

codecov bot commented Jan 24, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.94%. Comparing base (f262eda) to head (2f868ec).
Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5146   +/-   ##
=======================================
  Coverage   85.94%   85.94%           
=======================================
  Files          90       90           
  Lines       32718    32718           
=======================================
  Hits        28121    28121           
  Misses       4597     4597           
Flag Coverage Δ
wasm-lib 85.94% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@lf94 lf94 left a comment

Choose a reason for hiding this comment

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

Nice use of fromPromise, and good job migrating yet another action to actor :) I've approved in case you think my comments are not applicable / necessary

src/machines/modelingMachine.ts Outdated Show resolved Hide resolved
src/machines/modelingMachine.ts Outdated Show resolved Hide resolved
src/machines/modelingMachine.ts Outdated Show resolved Hide resolved
@franknoirot franknoirot force-pushed the franknoirot/adhoc/extrude-action-to-actor branch from cdbc788 to 68efa18 Compare January 27, 2025 17:09
@franknoirot franknoirot enabled auto-merge (squash) January 27, 2025 17:09
@lf94
Copy link
Contributor

lf94 commented Jan 27, 2025

Snapshots lying

@franknoirot franknoirot force-pushed the franknoirot/adhoc/extrude-action-to-actor branch from 4e2edf0 to 2f868ec Compare January 31, 2025 16:00
@franknoirot franknoirot merged commit d707c66 into main Jan 31, 2025
31 checks passed
@franknoirot franknoirot deleted the franknoirot/adhoc/extrude-action-to-actor branch January 31, 2025 18:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants