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

Screenshot example shouldn't use bevy_render directly #15317

Closed
alice-i-cecile opened this issue Sep 19, 2024 · 1 comment · Fixed by #15333
Closed

Screenshot example shouldn't use bevy_render directly #15317

alice-i-cecile opened this issue Sep 19, 2024 · 1 comment · Fixed by #15333
Labels
C-Examples An addition or correction to our examples D-Trivial Nice and easy! A great choice to get started with Bevy S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it!

Comments

@alice-i-cecile
Copy link
Member

          probably for another PR: this example shouldn't use bevy_render directly. we used to have a CI that errored when that happens

Originally posted by @mockersf in #15281 (comment)

Instead these should be bevy::render imports.

@alice-i-cecile alice-i-cecile added D-Trivial Nice and easy! A great choice to get started with Bevy C-Examples An addition or correction to our examples S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it! labels Sep 19, 2024
@mockersf
Copy link
Member

existing CI is

check-bevy-internal-imports:
runs-on: ubuntu-latest
timeout-minutes: 30
steps:
- uses: actions/checkout@v4
- name: Check for bevy_internal imports
shell: bash
run: |
errors=""
for file in $(find examples tests -name '*.rs'); do
if grep -q "use bevy_internal" "$file"; then
errors+="ERROR: Detected 'use bevy_internal' in $file\n"
fi
done
if [ -n "$errors" ]; then
echo -e "$errors"
echo " Avoid importing bevy_internal, it should not be used directly"
echo " Fix the issue by replacing 'bevy_internal' with 'bevy'"
echo " Example: 'use bevy::sprite::MaterialMesh2dBundle;' instead of 'bevy_internal::sprite::MaterialMesh2dBundle;'"
exit 1
fi

it currently only checks for bevy_internal imports, it could be extended to others

@alice-i-cecile alice-i-cecile added A-Build-System Related to build systems or continuous integration and removed A-Build-System Related to build systems or continuous integration labels Sep 19, 2024
github-merge-queue bot pushed a commit that referenced this issue Sep 20, 2024
# Objective

- Fixes #15319
- Fixes #15317

## Solution

- Updated CI task to check for _any_ `bevy_*` crates, rather than just
`bevy_internal`

---------

Co-authored-by: François Mockers <francois.mockers@vleue.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-Examples An addition or correction to our examples D-Trivial Nice and easy! A great choice to get started with Bevy 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.

2 participants