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

Fix doc warning in bevy_tasks #9348

Merged
merged 1 commit into from
Aug 5, 2023
Merged

Conversation

nicopap
Copy link
Contributor

@nicopap nicopap commented Aug 3, 2023

Objective

  • bevy_tasks emits warnings under certain conditions

When I run cargo clippy -p bevy_tasks the warning doesn't show up, while if I run it with cargo clippy -p bevy_asset the warning shows up.

Solution

  • Fix the warnings.

Longer term solution

We should probably fix CI so that those warnings do not slip through. But that's not the goal of this PR.

@nicopap nicopap added C-Docs An addition or correction to our documentation A-Tasks Tools for parallel and async work A-Diagnostics Logging, crash handling, error reporting and performance analysis labels Aug 3, 2023
@hymm
Copy link
Contributor

hymm commented Aug 3, 2023

the warning above can be fixed by replacing the line with

let temp_result = f.await;
result.borrow_mut().replace(temp_result);

@nicopap
Copy link
Contributor Author

nicopap commented Aug 3, 2023

For posterity, I had a section in the PR description showing a clippy lint related to async code, I removed it because I don't want it to be in the commit message.

the warning above can be fixed by replacing the line with

let temp_result = f.await;
result.borrow_mut().replace(temp_result);

Does indeed. I feel dumb. Thought it meant I shouldn't hold result beyond the .await.

@mockersf mockersf added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Aug 4, 2023
@mockersf mockersf added this pull request to the merge queue Aug 5, 2023
Merged via the queue into bevyengine:main with commit 60c6ca7 Aug 5, 2023
21 checks passed
@nicopap nicopap deleted the fix-clippy-sttp branch August 30, 2023 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Diagnostics Logging, crash handling, error reporting and performance analysis A-Tasks Tools for parallel and async work C-Docs An addition or correction to our documentation S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants