[Merged by Bors] - fix bevy_ecs macro path handling#1426
[Merged by Bors] - fix bevy_ecs macro path handling#1426jakobhellermann wants to merge 1 commit intobevyengine:mainfrom
Conversation
jakobhellermann
commented
Feb 10, 2021
jakobhellermann
commented
- It now doesn't search in the dev-dependencies anymore
- and the behaviour is consistent for derive_bundle and derive_system_param
|
How does this work if a user only uses the procedural macro in tests or examples? |
|
The use case where I ran into this was adding The only way it would break now is if someone has |
|
But to support that we could first try in regular dependencies and afterwards the dev-dependencies. Should I do that? |
|
I think so? I'm getting a massive sense of deja-vu - that is, I feel like we've already made this change somewhere else already, although I can't find it now. But yes, trying in regular dependencies first and then dev dependencies does make sense. |
|
In #1182 there was a similar change made to the reflect crate, to check regular dependencies first, and dev-dependencies second. |
|
I changed it. I think |
DJMcNab
left a comment
DJMcNab
left a comment
There was a problem hiding this comment.
Looks good. One small suggestion cart might want to merge, but equally this can also be merged as is.
crates/bevy_ecs/macros/src/lib.rs
Outdated
| }; | ||
| let crate_path: Path = syn::parse(path_str.parse::<TokenStream>().unwrap()).unwrap(); | ||
|
|
||
| let crate_path = bevy_ecs_path(); |
There was a problem hiding this comment.
Maybe ecs_crate or even just ecs would be better?
There was a problem hiding this comment.
Changed it to ecs.
There was a problem hiding this comment.
imo removing "path" from the name makes it harder to tell what the variable is. can we do ecs_path?
There was a problem hiding this comment.
I changed it to ecs_path.
|
Don't fix the clippy failures in this PR please - someone else will do that soon |
jakobhellermann
force-pushed
the
fix-derive-dependency
branch
from
67558a7 to
4cc8092
Compare
|
I don't know if you caught this but I added a couple of missing paths to the |
jakobhellermann
force-pushed
the
fix-derive-dependency
branch
from
4cc8092 to
557b38b
Compare
|
Thanks, fixed them now. |
jakobhellermann
force-pushed
the
fix-derive-dependency
branch
from
557b38b to
63a6d62
Compare
alice-i-cecile
added
C-Bug
|
resolve merge conflicts and I think this is good to go! |
jakobhellermann
force-pushed
the
fix-derive-dependency
branch
from
63a6d62 to
e4be688
Compare
- It now doesn't search in the dev-dependencies anymore - and the behaviour is consistent for derive_bundle and derive_system_param
jakobhellermann
force-pushed
the
fix-derive-dependency
branch
from
e4be688 to
d6b5c3e
Compare
|
done |
|
bors r+ |
- It now doesn't search in the dev-dependencies anymore - and the behaviour is consistent for derive_bundle and derive_system_param
|
Pull request successfully merged into main. Build succeeded: |
bors
bot
changed the title
It took me a little while to figure out how to use the `SystemParam` derive macro to easily create my own params. So I figured I'd add some docs and an example with what I learned. - Fixed a bug in the `SystemParam` derive macro where it didn't detect the correct crate name when used in an example (no longer relevant, replaced by #1426 - see further) - Added some doc comments and a short example code block in the docs for the `SystemParam` trait - Added a more complete example with explanatory comments in examples
6 participants
