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

Use workspace dependencies more. #6020

Merged
merged 1 commit into from
Jul 26, 2024

Conversation

waywardmonkeys
Copy link
Contributor

@waywardmonkeys waywardmonkeys commented Jul 23, 2024

Description
In the past, workspace dependencies couldn't be used everywhere due to complications in the Firefox environment. These are believed to be resolved and so we can try using them again to reduce version management overhead and duplicated information.

This patch makes some dependencies into workspace dependencies.

The bitflags dependency is changed to version 2.6 to match what was in naga rather than the bare 2 used elsewhere.

Testing
Compilation.

Checklist

  • Run cargo fmt.
  • Run cargo clippy. If applicable, add:
    • --target wasm32-unknown-unknown
    • --target wasm32-unknown-emscripten
  • Run cargo xtask test to run tests.
  • Add change to CHANGELOG.md. See simple instructions inside file. EDIT by @ErichDonGubler: Not necessary. 🙂

@waywardmonkeys waywardmonkeys requested review from a team as code owners July 23, 2024 06:35
@waywardmonkeys
Copy link
Contributor Author

This could be a draft and get updated as more dependencies are switched over, or we could use this to be sure that there aren't issues from enabling this.

wgpu-hal/Cargo.toml Outdated Show resolved Hide resolved
@Wumpf

This comment was marked as resolved.

@Wumpf Wumpf requested a review from ErichDonGubler July 23, 2024 08:09
@teoxoy

This comment was marked as resolved.

@ErichDonGubler

This comment was marked as resolved.

@ErichDonGubler
Copy link
Member

I'm going to merge this in a few hours, unless somebody has an objection (CC @gfx-rs/wgpu); I think the potential for conflicts is high, and I haven't felt or seen any controversy that what's here already is fine by itself.

@ErichDonGubler ErichDonGubler enabled auto-merge (squash) July 24, 2024 17:40
@ErichDonGubler ErichDonGubler added the area: infrastructure Testing, building, coordinating issues label Jul 24, 2024
auto-merge was automatically disabled July 24, 2024 17:49

Head branch was pushed to by a user without write access

@waywardmonkeys waywardmonkeys force-pushed the use-workspace-deps-more branch 3 times, most recently from 05c420f to 0c04ced Compare July 24, 2024 18:07
@waywardmonkeys
Copy link
Contributor Author

I've rebased this forward, added in the further work that I'd done.

@waywardmonkeys waywardmonkeys force-pushed the use-workspace-deps-more branch from 0c04ced to b5d7d94 Compare July 25, 2024 09:29
@teoxoy
Copy link
Member

teoxoy commented Jul 25, 2024

Should this close #3407? I think so. I don't think we want all the deps in the workspace toml, only the ones present in more than one crate.

@MarijnS95
Copy link
Contributor

Should this close #3407? I think so. I don't think we want all the deps in the workspace toml, only the ones present in more than one crate.

To me it seems like tracking all versions in one coherent place is nice. That way you also don't have to be aware of this and go move dependencies back and forth when they start being used in >1 place, or have their use removed and end up remaining used by just one crate.

@teoxoy
Copy link
Member

teoxoy commented Jul 25, 2024

That's fair, let's keep it open for the time being then.

@ErichDonGubler ErichDonGubler force-pushed the use-workspace-deps-more branch from b5d7d94 to 00a290c Compare July 26, 2024 16:12
@ErichDonGubler ErichDonGubler enabled auto-merge (squash) July 26, 2024 16:34
@ErichDonGubler
Copy link
Member

ErichDonGubler commented Jul 26, 2024

Just waiting on @gfx-rs/naga's approval here.

@ErichDonGubler ErichDonGubler merged commit 3166d37 into gfx-rs:trunk Jul 26, 2024
25 checks passed
@waywardmonkeys waywardmonkeys deleted the use-workspace-deps-more branch July 26, 2024 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: infrastructure Testing, building, coordinating issues
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

6 participants