Skip to content

Add [lints] table, fix adding #![allow(clippy::type_complexity)] everywhere#10011

Merged
alice-i-cecile merged 13 commits into
bevyengine:mainfrom
ameknite:lints-workspace
Nov 18, 2023
Merged

Add [lints] table, fix adding #![allow(clippy::type_complexity)] everywhere#10011
alice-i-cecile merged 13 commits into
bevyengine:mainfrom
ameknite:lints-workspace

Conversation

@ameknite

@ameknite ameknite commented Oct 3, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

Objective

Solution

[lints]
workspace = true

Changelog

  • Bump rust version to 1.74
  • Enable lints table for the workspace
[workspace.lints.clippy]
type_complexity = "allow"
  • Allow type complexity for all crates and examples
[lints]
workspace = true

@mockersf

mockersf commented Oct 4, 2023

Copy link
Copy Markdown
Member

blocked until Rust 1.74 is stable - 16 November, 2023

@mockersf mockersf added C-Code-Quality A section of code that is hard to understand or change S-Blocked This cannot move forward until something else changes labels Oct 4, 2023
@alice-i-cecile alice-i-cecile added this to the 0.13 milestone Oct 6, 2023
mogambro
mogambro reviewed Oct 11, 2023
View reviewed changes
Comment thread crates/bevy_animation/Cargo.toml Outdated
mnmaita
mnmaita reviewed Oct 13, 2023
View reviewed changes
Comment thread examples/ui/button.rs Outdated
Comment thread examples/tools/scene_viewer/scene_viewer_plugin.rs Outdated
Comment thread crates/bevy_transform/src/lib.rs Outdated
@ameknite ameknite force-pushed the lints-workspace branch 3 times, most recently from c6dfa17 to 0c7dd89 Compare November 6, 2023 02:02
@github-actions

github-actions Bot commented Nov 6, 2023

Copy link
Copy Markdown
Contributor

You added a new example but didn't add metadata for it. Please update the root Cargo.toml file.

2 similar comments
@github-actions

github-actions Bot commented Nov 6, 2023

Copy link
Copy Markdown
Contributor

You added a new example but didn't add metadata for it. Please update the root Cargo.toml file.

@github-actions

github-actions Bot commented Nov 6, 2023

Copy link
Copy Markdown
Contributor

You added a new example but didn't add metadata for it. Please update the root Cargo.toml file.

mnmaita
mnmaita reviewed Nov 6, 2023
View reviewed changes
Comment thread examples/3d/transmission.rs Outdated
Comment thread crates/bevy_reflect/bevy_reflect_derive/Cargo.toml Outdated
@alice-i-cecile alice-i-cecile removed the S-Blocked This cannot move forward until something else changes label Nov 16, 2023
@alice-i-cecile

alice-i-cecile commented Nov 16, 2023

Copy link
Copy Markdown
Member

No longer blocked!

@ameknite ameknite marked this pull request as ready for review November 16, 2023 18:28
cart
cart reviewed Nov 16, 2023
View reviewed changes
Comment thread crates/bevy_asset/Cargo.toml
Comment thread crates/bevy_audio/Cargo.toml Outdated
@hymm

hymm commented Nov 16, 2023

Copy link
Copy Markdown
Contributor

Should this bump the MSRV? I guess we're ok in CI since we're always using latest. Bevy contributors, I guess might get failures in CI that they don't see locally if they're using an older version. I would say we should bump it, but it's not necessarily required here.

@hymm hymm mentioned this pull request Nov 16, 2023
Closed
@13ros27

13ros27 commented Nov 16, 2023

Copy link
Copy Markdown
Contributor

I'm slightly unsure about using workspace lints in example files (definitely makes sense everywhere else) because it might confuse someone reading and copying the examples why their code is raising warnings and the example doesn't because its hidden back in the root Cargo.toml? This may be a non-issue because the fix for the user is clearly sign posted by the compiler but I thought it was worth mentioning.

@alice-i-cecile

alice-i-cecile commented Nov 16, 2023

Copy link
Copy Markdown
Member

Yeah, IMO we should bump the MSRV in this PR.

@ameknite

ameknite commented Nov 17, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@13ros27 yes that could happen, maybe we should keep the lint in the examples. I would like to hear the opinions of more people

This was referenced Nov 17, 2023
Closed
Closed
@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 Nov 18, 2023
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Nov 18, 2023
Merged via the queue into bevyengine:main with commit 951c9bb Nov 18, 2023
rdrpenguin04 pushed a commit to rdrpenguin04/bevy that referenced this pull request Jan 9, 2024
@ameknite @mnmaita
Add [lints] table, fix adding #![allow(clippy::type_complexity)] ev…
f7c2115 
…erywhere (bevyengine#10011)

# Objective

- Fix adding `#![allow(clippy::type_complexity)]` everywhere. like bevyengine#9796

## Solution

- Use the new [lints] table that will land in 1.74
(https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#lints)
- inherit lint to the workspace, crates and examples.
```
[lints]
workspace = true
```

## Changelog

- Bump rust version to 1.74
- Enable lints table for the workspace
```toml
[workspace.lints.clippy]
type_complexity = "allow"
```
- Allow type complexity for all crates and examples
```toml
[lints]
workspace = true
```

---------

Co-authored-by: Martín Maita <47983254+mnmaita@users.noreply.github.com>
salixsalicaceae pushed a commit to fslabs/bevy that referenced this pull request Mar 26, 2026
@ameknite @mnmaita
Add [lints] table, fix adding #![allow(clippy::type_complexity)] ev…
4cebda4 
…erywhere (bevyengine#10011)

# Objective

- Fix adding `#![allow(clippy::type_complexity)]` everywhere. like bevyengine#9796

## Solution

- Use the new [lints] table that will land in 1.74
(https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#lints)
- inherit lint to the workspace, crates and examples.
```
[lints]
workspace = true
```

## Changelog

- Bump rust version to 1.74
- Enable lints table for the workspace
```toml
[workspace.lints.clippy]
type_complexity = "allow"
```
- Allow type complexity for all crates and examples
```toml
[lints]
workspace = true
```

---------

Co-authored-by: Martín Maita <47983254+mnmaita@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cart cart cart left review comments
@alice-i-cecile alice-i-cecile alice-i-cecile approved these changes
+2 more reviewers
@mogambro mogambro mogambro left review comments
@mnmaita mnmaita mnmaita approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

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

Milestone

0.13

Development

Successfully merging this pull request may close these issues.

8 participants

@ameknite @mockersf @alice-i-cecile @hymm @13ros27 @cart @mnmaita @mogambro