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 document for experimental async programming support #411

Merged
merged 2 commits into from
Jan 12, 2025

Conversation

Guest0x0
Copy link
Collaborator

some example code requires JS FFI, so I tweak the CI workflow and only check for JS backend for those examples

Copy link

peter-jerry-ye-code-review bot commented Jan 10, 2025

‼️ This code review is generated by a bot. Please verify the content before trusting it.

Here are three potential issues or observations from the provided git diff output:

  1. Inconsistent Error Handling in Async Functions:

    • In the async_worker function, the suspend!! call is used with a callback that handles both normal resumption (resume_ok) and error resumption (resume_err). However, the error handling in the test block does not account for all possible errors. Specifically, the catch block in the test block only handles MyError, but the suspend!! call could potentially throw other types of errors. This could lead to unhandled exceptions in production code.
  2. Potential Issue with return in PowerShell Script:

    • In the PowerShell script section, the return statement is used to skip processing the async directory if the backend is not js. However, return in PowerShell exits the entire script block, which might not be the intended behavior. If the intention is to skip only the current iteration of the loop, continue should be used instead of return.
  3. Hardcoded Paths in Documentation:

    • The documentation includes hardcoded paths like next/sources/async/src/async.mbt. This could be problematic if the directory structure changes in the future. It might be better to use relative paths or a more flexible way to reference these files to avoid breaking the documentation when the project structure evolves.

These issues should be reviewed and addressed to ensure the robustness and maintainability of the code and documentation.

@Guest0x0 Guest0x0 force-pushed the Guest0x0/experimental-async-support branch 6 times, most recently from b7f0df4 to 35c8149 Compare January 10, 2025 08:57
@Guest0x0 Guest0x0 force-pushed the Guest0x0/experimental-async-support branch from 35c8149 to 707ca5d Compare January 10, 2025 09:48
@Guest0x0 Guest0x0 force-pushed the Guest0x0/experimental-async-support branch 2 times, most recently from 48c500a to b171505 Compare January 10, 2025 09:59
@Guest0x0 Guest0x0 force-pushed the Guest0x0/experimental-async-support branch 2 times, most recently from e96955c to ea1fb8c Compare January 10, 2025 10:01
@Guest0x0 Guest0x0 merged commit a4cc80c into main Jan 12, 2025
16 checks passed
@Guest0x0 Guest0x0 deleted the Guest0x0/experimental-async-support branch January 12, 2025 07:21
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.

1 participant