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 to async closures where possible #429

Open
GlenDC opened this issue Feb 21, 2025 · 5 comments
Open

migrate to async closures where possible #429

GlenDC opened this issue Feb 21, 2025 · 5 comments
Assignees
Labels
easy An easy issue to pick up for anyone. enhancement New feature or request good first issue Good for newcomers mentor available A mentor is available to help you through the issue.
Milestone

Comments

@GlenDC
Copy link
Member

GlenDC commented Feb 21, 2025

Most of the code, mostly examples and tests I imagine, in rama codebase which now uses the construct:

|...| async move {

can probably move to the async closures we now have access to since edition 2024 (MSRV 1.85)

async |...| {

and if move required:

async move |...| {
@GlenDC GlenDC added easy An easy issue to pick up for anyone. enhancement New feature or request good first issue Good for newcomers mentor available A mentor is available to help you through the issue. labels Feb 21, 2025
@GlenDC GlenDC added this to the v0.2 milestone Feb 21, 2025
@harshithsaiv
Copy link

Hi @GlenDC! I'd love to contribute to this issue and learn more in the process. Please let me know how I can get started, and kindly assign this issue to me. Looking forward to your guidance!

@GlenDC
Copy link
Member Author

GlenDC commented Feb 21, 2025

Most certainly, I've assigned it to you @harshithsaiv .

As a first time contributor please do take your time to read through https://github.com/plabayo/rama/blob/main/CONTRIBUTING.md, especially the tip about using just qa is useful as it will ensure that stuff like formatting and clippy lints are respected prior to pushing it to a a Pull Request (PR).

The way I would start is honestly just global search / grep through the codebase for patterns like:

| async move {

e.g.

grep -rn --include="*.rs" '| async move {' .

But in case you use a Graphical IDE you can probably achieve the same with a global search :)

@GlenDC
Copy link
Member Author

GlenDC commented Feb 21, 2025

There are also a couple that have the following:

grep -rn --include="*.rs" '| async {' .

Those should also be migrated to async functions, where possible.

Note that there are still valid use cases for async (move) { ... }, future blocks,
e.g. for low level bits where you wish to create a future only at the end of an otherwise sync function.
So there might very well still be patterns that match at the end of this task, which should only be the valid use cases left.

@harshithsaiv
Copy link

Thank you, @GlenDC! I appreciate the guidance. I'll go through the CONTRIBUTING.md file and start searching for occurrences of async move { as suggested. I'll update you if I have any questions.

@GlenDC
Copy link
Member Author

GlenDC commented Feb 21, 2025

Awesome. Do let me know if you need further guidance, or have any other questions. Also feel free to open a (draft) PR, even when not everything is working (yet). Especially useful in case you are stuck in any way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
easy An easy issue to pick up for anyone. enhancement New feature or request good first issue Good for newcomers mentor available A mentor is available to help you through the issue.
Projects
None yet
Development

No branches or pull requests

2 participants