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 Cargo workspace features for internal dependency versions #16652

Open
viridia opened this issue Dec 4, 2024 · 3 comments · May be fixed by #17486
Open

Use Cargo workspace features for internal dependency versions #16652

viridia opened this issue Dec 4, 2024 · 3 comments · May be fixed by #17486
Labels
A-Cross-Cutting Impacts the entire engine D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it!

Comments

@viridia
Copy link
Contributor

viridia commented Dec 4, 2024

Currently all of the Bevy sub-packages which depend on other parts of Bevy use explicit version numbers. This means that whenever a version of a package is bumped, you have to change the version number in every Cargo.toml that references it.

It would be much less work to use Cargo's workspace versioning feature:

bevy_app = { workspace = true }

This will use the version number specified in the top-level Cargo.toml (in the section workspace.dependencies).

Doing this means that the package version for each crate only needs to be specified in two places: in the package itself, and in the top-level workspace.dependencies. This will making bumping the versions much easier.

@viridia viridia added A-Cross-Cutting Impacts the entire engine D-Straightforward Simple bug fixes and API improvements, docs, test and examples labels Dec 4, 2024
@bushrat011899
Copy link
Contributor

I like this idea. This doesn't have the same issues that specifying external dependencies at a workspace-level does.

@mockersf
Copy link
Member

mockersf commented Dec 4, 2024

this is not a pain point as its handled by scripts and it already "just works"

@BD103
Copy link
Member

BD103 commented Dec 5, 2024

Workspace dependencies were broken when used with our compilation tests, last I checked, since they are technically in another workspace. If you get it working, though, I'm all for this!

this is not a pain point as its handled by scripts and it already "just works"

I think there's serious benefit in trying to make our codebase the best it can possibly can be, because it shows that we care about our work. Yes, there are people whose expertise mean that their time is better spent elsewhere, but there are also new, unfamiliar faces who can easily tackle this without needing to learn months of knowledge.

We can't achieve perfection, but there's virtue in trying to be improve, step by step.

@TimJentzsch TimJentzsch added the S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it! label Jan 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Cross-Cutting Impacts the entire engine D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants