Skip to content

Remove parking_lot dependency #20630

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 14 commits into from
Sep 30, 2025
Merged

Remove parking_lot dependency #20630

merged 14 commits into from
Sep 30, 2025

Conversation

@james7132
Copy link
Member

@james7132 james7132 commented Aug 18, 2025 •
edited
Loading

Objective

Spend less time compiling what is already in the standard library. Remove parking_lot from first party crate dependencies. Contribute to #18978. Supercedes #18996.

Solution

Use bevy_platform::sync types wherever possible. BevyManifest::shared directly returned a mapped guard, but that could be circumvented by just using a scope-like API instead.

The crate is still in the dependency tree, transitively, through wgpu, but is now no longer required for any non-rendering crates.

Testing

Ran tests locally.

Co-Authored-By: Zac Harrold zac@harrold.com.au

@james7132 james7132 added A-Assets Load files from disk to use for things like images, models, and sounds C-Dependencies A change to the crates that Bevy depends on M-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide D-Macros Code that generates Rust code labels Aug 18, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Aug 18, 2025

Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke!
You can review it at https://pixel-eagle.com/project/B04F67C0-C054-4A6F-92EC-F599FEC2FD1D?filter=PR-20630

If it's expected, please add the M-Deliberate-Rendering-Change label.

If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it.

1 similar comment
@github-actions
Copy link
Contributor

github-actions bot commented Aug 18, 2025

Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke!
You can review it at https://pixel-eagle.com/project/B04F67C0-C054-4A6F-92EC-F599FEC2FD1D?filter=PR-20630

If it's expected, please add the M-Deliberate-Rendering-Change label.

If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 18, 2025

Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke!
You can review it at https://pixel-eagle.com/project/B04F67C0-C054-4A6F-92EC-F599FEC2FD1D?filter=PR-20630

If it's expected, please add the M-Deliberate-Rendering-Change label.

If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it.

@james7132 james7132 added the S-Needs-Review Needs reviewer attention (from anyone!) to move forward label Aug 19, 2025
@james7132 james7132 mentioned this pull request Aug 19, 2025
Closed
@james7132 james7132 added this to the 0.18 milestone Aug 22, 2025
@james7132 james7132 mentioned this pull request Sep 6, 2025
Open
@alice-i-cecile alice-i-cecile added X-Uncontroversial This work is generally agreed upon D-Straightforward Simple bug fixes and API improvements, docs, test and examples labels Sep 9, 2025
andriyDev
andriyDev approved these changes Sep 13, 2025
View reviewed changes
Copy link
Contributor

@andriyDev andriyDev left a comment

Choose a reason for hiding this comment

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

Sorry for taking so long to get to this!

@james7132 james7132 added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Sep 13, 2025
@alice-i-cecile
Copy link
Member

alice-i-cecile commented Sep 29, 2025

@james7132, ping me once merge conflicts are resolved and I'll do a quick re-review + merge.

@james7132
Copy link
Member Author

james7132 commented Sep 30, 2025

@alice-i-cecile ready to go!

@alice-i-cecile alice-i-cecile added this pull request to the merge queue Sep 30, 2025
Merged via the queue into bevyengine:main with commit 329630c Sep 30, 2025
40 checks passed
@samoylovfp samoylovfp mentioned this pull request Oct 1, 2025
Merged
github-merge-queue bot pushed a commit that referenced this pull request Oct 1, 2025
@samoylovfp @alice-i-cecile @dloukadakis
Fix documentation for shared method in BevyManifest (#21312)
0b8844a 
Follow up to #20630.

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Dimitrios Loukadakis <dloukadakis@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alice-i-cecile alice-i-cecile alice-i-cecile approved these changes

+1 more reviewer

@andriyDev andriyDev andriyDev approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

A-Assets Load files from disk to use for things like images, models, and sounds C-Dependencies A change to the crates that Bevy depends on D-Macros Code that generates Rust code D-Straightforward Simple bug fixes and API improvements, docs, test and examples M-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it X-Uncontroversial This work is generally agreed upon

Projects

None yet

Milestone

0.18

Development

Successfully merging this pull request may close these issues.

3 participants

@james7132 @alice-i-cecile @andriyDev