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 for --force-warn in Clippy's driver run condition #9036

Merged
merged 1 commit into from
Jun 24, 2022

Conversation

xFrednet
Copy link
Member

Just a thing I've noticed while tinkering on the driver. Currently, the driver only checks for --cap-lints=allow to determine if Clippy should run on the code, but ignores possible --force-warn arguments


changelog: Others: Allowing all lints and only --force-warning some will now work with Clippy's driver

@rust-highfive
Copy link

r? @dswij

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jun 23, 2022
@xFrednet xFrednet force-pushed the 0000-force-warn-in-driver branch 2 times, most recently from dc95092 to af31c1e Compare June 23, 2022 08:01
src/driver.rs Outdated Show resolved Hide resolved
@dswij
Copy link
Member

dswij commented Jun 23, 2022

If I understand it correctly, will this change means that --force-warn clippy:: will completely precede --cap-lints allow?

@xFrednet
Copy link
Member Author

Yes, the --force-warn flag is defined to warn, even if --cap-lints allow is set. --force-warn clippy:: will be accepted by that implementation, but should issue a warning, since clippy:: is not a valid lint name. The whole cap_lints_allow is just a test to safe performance for dependencies, AFAIK.

Copy link
Member

@dswij dswij left a comment

Choose a reason for hiding this comment

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

Makes sense. LGTM, thanks for this!

@xFrednet
Copy link
Member Author

Thanks for the review! Is there anything else that needs to be done before the merge or did you forget to r+? 🙃

@dswij
Copy link
Member

dswij commented Jun 24, 2022

Oops, you're right 😅

@bors r+

@bors
Copy link
Collaborator

bors commented Jun 24, 2022

📌 Commit 4182803 has been approved by dswij

@bors
Copy link
Collaborator

bors commented Jun 24, 2022

⌛ Testing commit 4182803 with merge 3f47cd1...

@bors
Copy link
Collaborator

bors commented Jun 24, 2022

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: dswij
Pushing 3f47cd1 to master...

@bors bors merged commit 3f47cd1 into rust-lang:master Jun 24, 2022
@xFrednet xFrednet deleted the 0000-force-warn-in-driver branch June 24, 2022 10:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants