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

Integrate Clippy into the project #1724

Merged
merged 6 commits into from
Sep 26, 2023
Merged

Conversation

blyxyas
Copy link
Member

@blyxyas blyxyas commented Sep 20, 2023

This PR integrates Clippy into the project.
A user can run the clippy profile with a local Clippy build, with the --clippy flag targetting to the Clippy executable:

$ cargo run --bin collector  -- bench_local +nightly --profiles Clippy --clippy ../clippy/target/release/cargo-clippy

But I'm having some problems with the bot integration, so some feedback and tips in those files would be very appreciated.

  • Would the bot run this on every commit, where is it configured? (we don't want to run it every commit)
    Thanks for the feedback ❤️

Copy link
Contributor

@Kobzol Kobzol left a comment

Choose a reason for hiding this comment

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

Hi, thanks for the PR! I left some comments. I wonder what was your motivation for this PR? Do you work on Clippy and want to measure its performance on crates?

collector/src/bin/collector.rs Outdated Show resolved Hide resolved
collector/src/compile/benchmark/profile.rs Show resolved Hide resolved
collector/src/toolchain.rs Outdated Show resolved Hide resolved
collector/src/toolchain.rs Outdated Show resolved Hide resolved
collector/src/toolchain.rs Outdated Show resolved Hide resolved
site/src/request_handlers/github.rs Outdated Show resolved Hide resolved
database/src/pool/sqlite.rs Outdated Show resolved Hide resolved
@blyxyas
Copy link
Member Author

blyxyas commented Sep 21, 2023

I wonder what was your motivation for this PR? Do you work on Clippy and want to measure its performance on crates?

Yep, we wanted a tool for measuring Clippy's performance on crates. Not for every commit, but to run once every 7 days or when triggered by a maintainer (We don't need a lot of active monitoring + Clippy benchmarking is really resource-intensive)

@blyxyas
Copy link
Member Author

blyxyas commented Sep 22, 2023

This new commit should be just like the first one, but discarding anything server-related =^w^=

Copy link
Contributor

@Kobzol Kobzol left a comment

Choose a reason for hiding this comment

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

Thanks for the changes. Some problems that cause CI to fail still have to be resolved, as per the other comments.

Also, as per #1724 (comment), we shouldn't include Clippy in Profile::all() for now. Let's just remove it from there, that should fix collection on the server. Later we might add it back once we're sure that clippy benchmarks can be run on the perf server.

collector/src/toolchain.rs Outdated Show resolved Hide resolved
collector/src/toolchain.rs Outdated Show resolved Hide resolved
@Kobzol Kobzol changed the title Integrate Clippy into the project (Help needed) Integrate Clippy into the project Sep 25, 2023
Copy link
Contributor

@Kobzol Kobzol left a comment

Choose a reason for hiding this comment

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

Great, thanks for the changes! :)

collector/src/toolchain.rs Outdated Show resolved Hide resolved
@Kobzol Kobzol merged commit 2a9fade into rust-lang:master Sep 26, 2023
9 checks passed
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.

None yet

2 participants