Skip to content

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

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 13 commits into from
Nov 18, 2023
Merged

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

merged 13 commits into from
Nov 18, 2023

Conversation

@ameknite
Copy link
Contributor

@ameknite ameknite commented Oct 3, 2023 •
edited
Loading

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
Copy link
Member

mockersf commented Oct 4, 2023

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
mnmaita
mnmaita reviewed Oct 13, 2023
View reviewed changes
@ameknite ameknite force-pushed the lints-workspace branch 3 times, most recently from c6dfa17 to 0c7dd89 Compare November 6, 2023 02:02
@github-actions
Copy link
Contributor

github-actions bot commented Nov 6, 2023

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

2 similar comments
@github-actions
Copy link
Contributor

github-actions bot commented Nov 6, 2023

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

@github-actions
Copy link
Contributor

github-actions bot commented Nov 6, 2023

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
@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
Copy link
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
@hymm
Copy link
Contributor

hymm commented Nov 16, 2023

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
Copy link
Contributor

13ros27 commented Nov 16, 2023

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
Copy link
Member

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

@ameknite
Copy link
Contributor Author

ameknite commented Nov 17, 2023 •
edited
Loading

@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 <[email protected]>
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