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: use async fns in minimal example #7600

Merged
merged 3 commits into from
Apr 12, 2024
Merged

Conversation

onbjerg
Copy link
Collaborator

@onbjerg onbjerg commented Apr 12, 2024

Converts the minimal example to showcases that an ExEx is just async fn(ctx) -> future and that you can use async fns for both, allowing you to construct ExEx's without implementing Future manually

@onbjerg onbjerg added A-exex Execution Extensions C-example Examples labels Apr 12, 2024
@onbjerg onbjerg requested a review from gakonst as a code owner April 12, 2024 14:36
Copy link
Member

@DaniPopes DaniPopes left a comment

Choose a reason for hiding this comment

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

Can we convert the other examples to use async fn instead of impl Future? This is much more readable

@gakonst
Copy link
Member

gakonst commented Apr 12, 2024

totally agree with dani, unless there's some non obvious advantage here.

does this also mean we can do 2 exex's that are tokio::join'ed?

@onbjerg
Copy link
Collaborator Author

onbjerg commented Apr 12, 2024

I think if we want to convert it all to async fns, I'm going to scrap async-fn and just convert minimal to an async fn, then in follow ups convert the rest

@DaniPopes
Copy link
Member

sgtm

@onbjerg onbjerg changed the title feat: add async fn exex example refactor: use async fns in minimal example Apr 12, 2024
Copy link
Collaborator

@shekhirin shekhirin left a comment

Choose a reason for hiding this comment

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

bullish

@onbjerg onbjerg force-pushed the onbjerg/exex-async-fn-example branch from ed50134 to 9779191 Compare April 12, 2024 14:53
@onbjerg onbjerg enabled auto-merge April 12, 2024 14:55
@onbjerg onbjerg added this pull request to the merge queue Apr 12, 2024
Merged via the queue into main with commit d950bce Apr 12, 2024
27 checks passed
@onbjerg onbjerg deleted the onbjerg/exex-async-fn-example branch April 12, 2024 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-exex Execution Extensions C-example Examples
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants