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

Import our intended set of lints #432

Merged
merged 14 commits into from
Jul 18, 2024
Merged

Conversation

DJMcNab
Copy link
Member

@DJMcNab DJMcNab commented Jul 17, 2024

See discussion in #linebender > Standard Lint set

Of note: I have taken steps to ensure that this can be practically reviewed by not applying most of the lints.
The commented out lints make good follow-ups

@DJMcNab DJMcNab requested a review from PoignardAzur July 17, 2024 11:22
Copy link
Contributor

@PoignardAzur PoignardAzur left a comment

Choose a reason for hiding this comment

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

LGTM.

masonry/src/util.rs Outdated Show resolved Hide resolved
@DJMcNab DJMcNab force-pushed the lints-glorious-lints branch from ea88aa1 to 4759177 Compare July 18, 2024 08:40
@DJMcNab DJMcNab enabled auto-merge July 18, 2024 08:54
@DJMcNab DJMcNab added this pull request to the merge queue Jul 18, 2024
Merged via the queue into linebender:main with commit f11b648 Jul 18, 2024
16 checks passed
@DJMcNab DJMcNab deleted the lints-glorious-lints branch July 18, 2024 09:05
rust.unused_macro_rules = "warn"
rust.variant_size_differences = "warn"

clippy.allow_attributes_without_reason = "warn"
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm this, pollutes my diagnostics in my editor, without being able to action about it, since this does seem to be experimental, and thus CI fails when adding a reason (as seen here) , should we disable that for now, and enable it, when it's stable, maybe with directly fixing all of this, so that it doesn't contribute to OCD^^?

Copy link
Member Author

@DJMcNab DJMcNab Jul 19, 2024

Choose a reason for hiding this comment

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

We can comment this out.

However, the behaviour you're seeing is confusing - since rust-lang/rust-clippy#12999, this shouldn't be happening for you, because it checks the MSRV to see whether to suggest this lint

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm that's interesting (just updated nightly, and still the issue).
I don't use rust via rustup, but via rust-overlay maybe that's an issue, as there's no rustup-binary on my system. I mean it's not critical to me, I can just comment it out locally.

github-merge-queue bot pushed a commit that referenced this pull request Jul 19, 2024
Discovered in #432; this would otherwise cause errors for users of
nightly (or 1.81 beta, due to be released next week)
github-merge-queue bot pushed a commit to linebender/peniko that referenced this pull request Aug 6, 2024
Increases the MSRV to allow setting lints in `Cargo.toml`.

This imports the same set of lints as in
linebender/xilem#432
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants