-
-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Fix inconsistent strum dependencies and imports #40907
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
Conversation
|
Wouldn't it make more sense to simply add derive feature to strum in workspace toml, as I think nearly all uses of strum uses the macros? |
|
Would it also be possible to move the derive feature to the root |
|
I usually prefer to be explicit about things like this (explicit deps, etc), but currently anything which fixes |
|
Let me try it the other way around. |
|
Well, compositing_traits only need strum_macros: |
The `scripts_traits` crate was the only crate depending on `strum` with the `derive` feature. This accidentally allowed other crates to import strum macros via `strum::` without declaring their own dependency on `strum_macros`, causing compilation issues when running `./mach test-unit -p net`. This patch makes the imporys consistent across the codebase by: - replacing all `strum_macro::` imports with `strum::` imports - removing strum_macro dependencies - adding derive feature to the strum workspace Signed-off-by: Jan Varga <jvarga@igalia.com>
|
Ok, I reworked it as suggested. |
mrobinson
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. This is a nice cleanup.
The
scripts_traitscrate was the only crate depending onstrumwith thederivefeature. This accidentally allowed other crates to import strummacros via
strum::without declaring their own dependency onstrum_macros,causing compilation issues when running
./mach test-unit -p net.This PR makes the imports consistent across the code base by:
strum_macro::imports withstrum::importsTesting: Unit tests continue to pass