-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Change bevy_internal to be a 'thinner facade' #2851
Conversation
This change should (hopefully) fix the [src] links for the bevy crates on docs.rs
That makes it easy to forget to add a new bevy crate as bevy_dylib dependency. This will likely go unnoticed in most cases, and only causing slower link times and when using dylibs yourself possibly an error about dependencies being included in more than one dylib at the same time. |
crates/bevy_log/Cargo.toml
Outdated
[dev-dependencies] | ||
bevy_internal = { path = "../bevy_internal", version = "0.5.0" } | ||
# [dev-dependencies] | ||
# bevy_internal = { path = "../bevy_internal", version = "0.5.0" } |
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.
Could you please remove this?
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.
Sure - I was checking out whether bevy_internal
was used, and it's used for a doctest in here.
Not sure why I even commented it out to be honest...
mod default_plugins; | ||
#[doc(hidden)] |
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.
Does #![doc(hidden)]
at the top of the file work? And should it even be hidden at all if most users likely won't look at the docs of this crate?
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.
I'm not sure, maybe it not being hidden would be fine? The #![doc(hidden)]
at the top of the file would also hide MinimalPlugins
and DefaultPlugins
I did some tests on this PR (commit: f946e84) Documenting
|
Co-authored-by: r00ster <r00ster91@protonmail.com>
I'm going to close this for now; I think we can do better, but just diving in without creating a model isn't feasible. |
None of them are remotely necessary, if we import the trick from bevyengine#2851 This works around rust-lang/rust-analyzer#11410
# Objective - I think our codebase is hit badly by rust-lang/rust-analyzer#11410 - None of our uses of cyclic dependencies are remotely necessary - Note that these are false positives in rust-analyzer, however it's probably easier for us to work around this - Note also that I haven't confirmed that this is causing rust-analyzer to not work very well, but it's not a bad guess. ## Solution - Remove our cyclic dependencies - Import the trick from #2851 for no-op plugin groups.
# Objective - I think our codebase is hit badly by rust-lang/rust-analyzer#11410 - None of our uses of cyclic dependencies are remotely necessary - Note that these are false positives in rust-analyzer, however it's probably easier for us to work around this - Note also that I haven't confirmed that this is causing rust-analyzer to not work very well, but it's not a bad guess. ## Solution - Remove our cyclic dependencies - Import the trick from bevyengine#2851 for no-op plugin groups.
# Objective - I think our codebase is hit badly by rust-lang/rust-analyzer#11410 - None of our uses of cyclic dependencies are remotely necessary - Note that these are false positives in rust-analyzer, however it's probably easier for us to work around this - Note also that I haven't confirmed that this is causing rust-analyzer to not work very well, but it's not a bad guess. ## Solution - Remove our cyclic dependencies - Import the trick from bevyengine#2851 for no-op plugin groups.
# Objective - I think our codebase is hit badly by rust-lang/rust-analyzer#11410 - None of our uses of cyclic dependencies are remotely necessary - Note that these are false positives in rust-analyzer, however it's probably easier for us to work around this - Note also that I haven't confirmed that this is causing rust-analyzer to not work very well, but it's not a bad guess. ## Solution - Remove our cyclic dependencies - Import the trick from bevyengine#2851 for no-op plugin groups.
This change should (hopefully) fix the
[src]
links for the bevy crates ondocs.rs
Objective
bevy::app::App
, which doesn't have a[src]
linkbevy_internal::app::App
, which does have a[src]
linkSolution
[src]
links into thebevy
crate, as well as the prelude.bevy
is the user facing crate.bevy_internal
still needs to exist for dynamic linking reasons - in particular so thatDefault/MinimalPlugins
still get dynamically linked.bevy_internal
entirely, and depend on the crates directly inbevy_dylib