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

Let Cargo track CLIPPY_ARGS #6834

Merged
merged 4 commits into from Mar 8, 2021
Merged

Let Cargo track CLIPPY_ARGS #6834

merged 4 commits into from Mar 8, 2021

Conversation

ghost
Copy link

@ghost ghost commented Mar 3, 2021

This PR makes clippy-driver emit CLIPPY_ARGS in its dep-info output.

Just like #6441, this allows this workflow to work:

cargo clippy # warning: empty `loop {}` wastes CPU cycles
cargo clippy -- -A clippy::empty_loop # no warnings emitted

But without rebuilding all dependencies.

cc https://rust-lang.zulipchat.com/#narrow/stream/257328-clippy/topic/CLIPPY_ARGS.20is.20not.20tracked.20by.20Cargo/near/228599088

changelog: Cargo now re-runs Clippy if arguments after -- provided to cargo clippy are changed.

@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @phansch (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Mar 3, 2021
src/driver.rs Show resolved Hide resolved
@ghost

This comment has been minimized.

@phansch
Copy link
Member

phansch commented Mar 8, 2021

Thank you for the PR, the dogfood test changes look good to me! However, given our history with driver changes and my lack of knowledge here, I'd like to get a second pair of eyes on this.

Maybe r? @flip1995

@rust-highfive rust-highfive assigned flip1995 and unassigned phansch Mar 8, 2021
src/driver.rs Outdated Show resolved Hide resolved
src/driver.rs Outdated Show resolved Hide resolved
tests/dogfood.rs Outdated Show resolved Hide resolved
src/driver.rs Show resolved Hide resolved
@flip1995
Copy link
Member

flip1995 commented Mar 8, 2021

Local testing didn't lead to any obvious problems with this, so I'm happy to merge this. But we should revisit the placement of the depinfo change, which will most likely require a rustc change.

@bors r=phansch,flip1995,oli-obk

@bors
Copy link
Collaborator

bors commented Mar 8, 2021

📌 Commit 3cd5f44 has been approved by phansch,flip1995,oli-obk

@bors
Copy link
Collaborator

bors commented Mar 8, 2021

⌛ Testing commit 3cd5f44 with merge 3dc2a58...

bors added a commit that referenced this pull request Mar 8, 2021
Let Cargo track CLIPPY_ARGS

This PR makes `clippy-driver` emit `CLIPPY_ARGS` in its `dep-info` output.

Just like #6441, this allows this workflow to work:
```shell
cargo clippy # warning: empty `loop {}` wastes CPU cycles
cargo clippy -- -A clippy::empty_loop # no warnings emitted
```
But without rebuilding all dependencies.

cc https://rust-lang.zulipchat.com/#narrow/stream/257328-clippy/topic/CLIPPY_ARGS.20is.20not.20tracked.20by.20Cargo/near/228599088

Changelog: Cargo now re-runs Clippy if arguments after `--` provided to `cargo clippy` are changed.
@bors
Copy link
Collaborator

bors commented Mar 8, 2021

💔 Test failed - checks-action_test

@flip1995
Copy link
Member

flip1995 commented Mar 8, 2021

@bors retry

@bors
Copy link
Collaborator

bors commented Mar 8, 2021

⌛ Testing commit 3cd5f44 with merge d0d5232...

@bors
Copy link
Collaborator

bors commented Mar 8, 2021

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: phansch,flip1995,oli-obk
Pushing d0d5232 to master...

@bors bors merged commit d0d5232 into rust-lang:master Mar 8, 2021
@ghost ghost deleted the clippy-args branch March 8, 2021 13:35
@ghost
Copy link
Author

ghost commented Mar 8, 2021

But we should revisit the placement of the depinfo change, which will most likely require a rustc change.

Should I open a https://github.com/rust-lang/rust issue for this?

@flip1995
Copy link
Member

flip1995 commented Mar 8, 2021

Yeah, I think that would be good 👍

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.

5 participants