Skip to content

Conversation

@tonyfettes
Copy link
Contributor

No description provided.

@tonyfettes tonyfettes requested a review from rami3l April 22, 2025 02:10
@peter-jerry-ye-code-review
Copy link

peter-jerry-ye-code-review bot commented Apr 22, 2025

Async function type signatures are more explicit

Category
Maintainability
Code Snippet
pub fn async_run(f : () -> Unit!Async) -> Unit
Recommendation
The change from async () -> Unit to () -> Unit!Async is good. Keep this pattern consistent throughout the codebase.
Reasoning
The new syntax makes it clearer that async is an effect (like Error) rather than a modifier on the function type. This improves type system clarity and consistency.

Simplified async call syntax

Category
Correctness
Code Snippet
op!!() -> op!()
Recommendation
The change from double bang (!!) to single bang (!) for async calls is good. Continue migrating all async calls to use single bang notation.
Reasoning
The simpler syntax reduces visual noise while maintaining the semantic meaning that this is an async operation. Double bangs could be confused with non-null assertions in other languages.

Consistent error handling with Error effect

Category
Maintainability
Code Snippet
pub fn Promise::unsafe_new[T](op : () -> T!Error + Async) -> Promise
Recommendation
Good change to explicitly specify Error effect. Consider adding documentation explaining the Error+Async effect combination.
Reasoning
Making the Error effect explicit in the type signature helps developers understand potential failure modes and required error handling. The combination with Async effect clearly shows this is an asynchronous operation that may fail.

@rami3l
Copy link
Contributor

rami3l commented Apr 22, 2025

@tonyfettes Thanks! However it looks like there is another group of deprecated syntax to be fixed?

@rami3l rami3l merged commit a81f525 into moonbit-community:main Apr 24, 2025
4 checks passed
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.

2 participants