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

Check cfg during CI and fix feature typos #12103

Merged
merged 8 commits into from
Feb 25, 2024
Merged

Conversation

eerii
Copy link
Contributor

@eerii eerii commented Feb 24, 2024

Objective

Solution

  • Create a new cfg-check to the CI that runs cargo check -Zcheck-cfg --workspace using cargo nightly (and fails if there are warnings)
  • Fix all warnings generated by the new check

Changelog

  • Remove all redundant imports
  • Fix cfg wasm32 targets
  • Add 3 dead code exceptions (should StandardColor be unused?)
  • Convert ios_simulator to a feature (I'm not sure if this is the right way to do it, but the check complained before)

Migration Guide

No breaking changes

@eerii eerii marked this pull request as ready for review February 24, 2024 23:43
@alice-i-cecile alice-i-cecile added C-Bug An unexpected or incorrect behavior A-Build-System Related to build systems or continuous integration C-Code-Quality A section of code that is hard to understand or change labels Feb 24, 2024
@alice-i-cecile alice-i-cecile requested a review from BD103 February 24, 2024 23:47
@alice-i-cecile
Copy link
Member

Wow, this is an incredibly useful check. There are some nasty subtle bugs in there.

Copy link
Member

@james7132 james7132 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great. I'm a bit concerned about false positives being a bit too aggressive, but this is really useful.

Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once we remove the dead code from bevy_reflect this LGTM.

@eerii eerii requested a review from alice-i-cecile February 25, 2024 11:29
@alice-i-cecile alice-i-cecile 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 Feb 25, 2024
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Feb 25, 2024
Merged via the queue into bevyengine:main with commit 5f8f3b5 Feb 25, 2024
28 of 29 checks passed
msvbg pushed a commit to msvbg/bevy that referenced this pull request Feb 26, 2024
# Objective

- Add the new `-Zcheck-cfg` checks to catch more warnings
- Fixes bevyengine#12091

## Solution

- Create a new `cfg-check` to the CI that runs `cargo check -Zcheck-cfg
--workspace` using cargo nightly (and fails if there are warnings)
- Fix all warnings generated by the new check

---

## Changelog

- Remove all redundant imports
- Fix cfg wasm32 targets
- Add 3 dead code exceptions (should StandardColor be unused?)
- Convert ios_simulator to a feature (I'm not sure if this is the right
way to do it, but the check complained before)

## Migration Guide

No breaking changes

---------

Co-authored-by: Alice Cecile <[email protected]>
msvbg pushed a commit to msvbg/bevy that referenced this pull request Feb 26, 2024
# Objective

- Add the new `-Zcheck-cfg` checks to catch more warnings
- Fixes bevyengine#12091

## Solution

- Create a new `cfg-check` to the CI that runs `cargo check -Zcheck-cfg
--workspace` using cargo nightly (and fails if there are warnings)
- Fix all warnings generated by the new check

---

## Changelog

- Remove all redundant imports
- Fix cfg wasm32 targets
- Add 3 dead code exceptions (should StandardColor be unused?)
- Convert ios_simulator to a feature (I'm not sure if this is the right
way to do it, but the check complained before)

## Migration Guide

No breaking changes

---------

Co-authored-by: Alice Cecile <[email protected]>
github-merge-queue bot pushed a commit that referenced this pull request Mar 6, 2024
# Objective

- #12103 broke iOS simulator support, it doesn't even compile anymore

## Solution

- Fix the feature
spectria-limina pushed a commit to spectria-limina/bevy that referenced this pull request Mar 9, 2024
# Objective

- bevyengine#12103 broke iOS simulator support, it doesn't even compile anymore

## Solution

- Fix the feature
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Build-System Related to build systems or continuous integration C-Bug An unexpected or incorrect behavior C-Code-Quality A section of code that is hard to understand or change S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Check cfg to catch feature typos
5 participants