-
-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Add README.md
to all crates
#13184
Merged
Merged
Add README.md
to all crates
#13184
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Cargo automatically detects `README.md` files, so manually specifying it is not necessary. This is for consistency with the rest of the crates.
My feelings on this are not strong. If someone objects, I'd be willing to revert it. (Mainly because it's difficult to define what is "less common." gLTF isn't expanded, but Ptr is.)
BD103
added
C-Docs
An addition or correction to our documentation
A-Cross-Cutting
Impacts the entire engine
labels
May 2, 2024
In follow-ups, I'd like to:
I'm mainly splitting this up because the above could be more contentious. |
jgayfer
approved these changes
May 2, 2024
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.
Looks reasonable. I'm okay with expanding the acronyms; they're more clear for someone new to the project like I am.
NthTensor
approved these changes
May 2, 2024
NthTensor
added
the
S-Ready-For-Final-Review
This PR has been approved by the community. It's ready for a maintainer to consider merging it
label
May 2, 2024
IceSentry
approved these changes
May 2, 2024
github-merge-queue bot
pushed a commit
that referenced
this pull request
May 3, 2024
# Objective - Follow-up of #13184 :) - We use `ui_test` to test compiler errors for our custom macros. - There are four crates related to compile fail tests - `bevy_ecs_compile_fail_tests`, `bevy_macros_compile_fail_tests`, and `bevy_reflect_compile_fail_tests`, which actually test the macros. - [`bevy_compile_test_utils`](https://github.com/bevyengine/bevy/tree/64c1c65783938facc59d9b36cbaa6deba435d84e/crates/bevy_compile_test_utils), which provides helpers and common patterns for these tests. - All of these crates reside within the `crates` directory. - This can be confusing, especially for newcomers. All of the other folders in `crates` are actual published libraries, except for these 4. ## Solution - Move all compile fail tests to a `compile_fail` folder under their corresponding crate. - E.g. `crates/bevy_ecs_compile_fail_tests` would be moved to `crates/bevy_ecs/compile_fail`. - Move `bevy_compile_test_utils` to `tools/compile_fail_utils`. There are a few benefits to this approach: 1. An internal testing detail is less intrusive (and confusing) for those who just want to browse the public Bevy interface. 2. Follows a pre-existing approach of organizing related crates inside a larger crate's folder. - See `bevy_gizmos/macros` for an example. 4. Makes consistent the terms `compile_test`, `compile_fail`, and `compile_fail_test` in code. It's all just `compile_fail` now, because we are specifically testing the error messages on compiler failures. - To be clear it can still be referred to by these terms in comments and speech, just the names of the crates and the CI command are now consistent. ## Testing Run the compile fail CI command: ```shell cargo run -p ci -- compile-fail ``` If it still passes, then my refactor was successful.
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
C-Docs
An addition or correction to our documentation
S-Ready-For-Final-Review
This PR has been approved by the community. It's ready for a maintainer to consider merging it
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Objective
README.md
is a common file that usually gives an overview of the folder it is in.README.md
is rendered as the main description.README.md
files, which makes it more difficult to understand their purpose.README.md
files that this PR and its follow-ups intend to fix.Solution
README.md
file for all crates that do not have one.readme
field inCargo.toml
forbevy
andbevy_reflect
.README.md
files. The field is only there if you name it something else, likeINFO.md
.bevy_utils
'sREADME.md
.Readme.md
, which is inconsistent with the rest of the project.README.md
, because Git appears to be case-insensitive.bevy_ptr
andbevy_utils
.README.md
files, I preferred using expanded acronyms in the titles. (E.g. "Bevy Developer Tools" instead of "Bevy Dev Tools".)README.md
files to follow the same scheme.bevy_time
andbevy_transform
, which are the only crates currently lacking them.Changelog
README.md
files to all crates missing it.