Script that runs the lints on most popular crates #207
Conversation
| for (i, version_i0) in versions.iter().enumerate() { | ||
| if i + 1 < versions.len() { | ||
| let version_i1 = &versions[i + 1]; | ||
| if version_i0.yanked && !version_i1.yanked { // it gives more interesting results for testing |
There was a problem hiding this comment.
This would certainly give interesting results, but aren't we primarily interested in checking the most recent version vs its baseline? I'd assume we want to make sure:
- new crates can adopt cargo-semver-checks without running into any errors
- we don't keep re-checking mostly-the-same code over and over again, and instead make better use of our limited CPU time
I think this is fine to have as a mode behind a flag, but I think the primary mode of this tool would just be to check the most recent "normal" version of each crate against its baseline.
There was a problem hiding this comment.
Removed yanked releases in 4f46322.
It nerveless didn't work, because cargo doesn't allow setting a yanked release as a dependency.
There was a problem hiding this comment.
I plan to add --all-releases and --only-latest-release as commands when the script will work.
| save_manifest(create_rustdoc_manifest_for_crate_version(&crate_info.name, &version_i0.num, &version_i0.features), "crate_current/Cargo.toml")?; | ||
| save_manifest(create_rustdoc_manifest_for_crate_version(&crate_info.name, &version_i1.num, &version_i1.features), "crate_baseline/Cargo.toml")?; | ||
| run_checks_on_manifest()?; |
There was a problem hiding this comment.
With this control flow through the ? operators, won't all this immediately break as soon as some crate fails to build and generate rustdoc? This happens in a noticeable fraction of all crates, due to things like missing binary dependencies for example, and I think we want to catch and report that error rather than stopping the entire run over it.
There was a problem hiding this comment.
Right. When the script will work I'll rewrite this part to log an appropriate information and skip this particular crate.
| @@ -0,0 +1,13 @@ | |||
| [package] | |||
There was a problem hiding this comment.
I don't think we want to put an entire crate in scripts. Future maintainers of this project would find that very confusing, and it's not in line with how Rust projects tend to be structured. Consider moving the crate into its own top-level directory.
There was a problem hiding this comment.
Can you suggest a good name for the crate?
There was a problem hiding this comment.
Maybe check-semver-of-registry-crates?
There was a problem hiding this comment.
Perhaps registry-semver-scanner? Or, a different direction, semver-crater as a riff on crater, the tool that tests new Rust releases against all registry crates?
check-semver-of-registry-crates is a bit long, especially for something we may need to type in.
Also, depending on the discussion in #86, you may be able to move this to src/bin/ and configure the Cargo.toml to gate this code behind a new unstable feature.
tonowak
force-pushed
the
script_run_on_popular_crates
branch
from
5dfa9ee
to
cb43a45
Compare
tonowak
force-pushed
the
script_run_on_popular_crates
branch
from
95e17f6
to
cc007c8
Compare
tonowak
force-pushed
the
script_run_on_popular_crates
branch
from
cc007c8
to
ca77a4b
Compare
|
This PR is now stacked on #314. (damn, I made a mess with the big list of commits, ugh rebases) |
src/bin/semver-crater.rs
Outdated
| let mut filtered: Vec<Version> = vec![versions | ||
| .first() | ||
| .expect("received empty list of versions") | ||
| .clone()]; | ||
| filtered.extend(versions.windows(3).filter_map(|vec| { | ||
| let mut iter = vec.iter(); | ||
| let version_v0 = semver::Version::parse(&iter.next().unwrap().num).unwrap(); | ||
| let v1 = iter.next().unwrap(); | ||
| let version_v2 = semver::Version::parse(&iter.next().unwrap().num).unwrap(); | ||
| assert!(iter.next().is_none()); | ||
|
|
||
| let keep_middle = match self { | ||
| Self::CheckAllReleases => true, | ||
| Self::SkipAdjacentPatches => { | ||
| version_v0.major != version_v2.major || version_v0.minor != version_v2.minor | ||
| } | ||
| Self::SkipAdjacentMinor => version_v0.major != version_v2.major, | ||
| }; | ||
| match keep_middle { | ||
| true => Some(v1.clone()), | ||
| false => None, | ||
| } | ||
| })); | ||
| filtered.push( | ||
| versions | ||
| .last() | ||
| .expect("received empty list of versions") | ||
| .clone(), | ||
| ); | ||
| filtered |
There was a problem hiding this comment.
I won't deny -- this code is ugly as hell. But it works. And I don't really have any ideas on how to make it nicer.
|
The CLI needs some polishing, but I'm not experienced in stuff like that -- @obi1kenobi can you review it? I'm thinking that Assuming |
tonowak
force-pushed
the
script_run_on_popular_crates
branch
from
3badf18
to
a3a236a
Compare
src/bin/semver-crater.rs
Outdated
| #[arg(short, long)] | ||
| crates: Vec<String>, |
There was a problem hiding this comment.
I'm not sure, how to make it clear that some arguments run the tool on all crates (and then another argument like "crate_count_limit" would be used to tell semver-crater how many crates to check) and some arguments are so that the tool runs only on the selected crates?
tonowak
force-pushed
the
script_run_on_popular_crates
branch
from
a3a236a
to
5adbdd7
Compare
…obi#420) * Switch to dtolnay/rust-toolchain action for setting up Rust. * Minor fixes.
tonowak
force-pushed
the
script_run_on_popular_crates
branch
from
5adbdd7
to
45cc338
Compare
tonowak
force-pushed
the
script_run_on_popular_crates
branch
from
45cc338
to
91f7816
Compare
It downloads data from crates.io about the most downloaded crates and its versions, and then runs cargo-semver-checks on those crates to find false-positives or bugs in the tool.