Open conradludgate opened 10 months ago
That is very cool. I am generally very hesitant to add new dependencies to CI. CI failing due to breakages or behaviors I don't understand from other tools is one of my bigger time sinks. Given that I think this has been the only accidental semver violation in memchr
in its long history, I'm not sure it warrants the extra dependency in CI.
👋 Hi! I'm Predrag, and I maintain cargo-semver-checks
.
Of course this is your crate and you should do what you think is best. If there's any info I could give you that might change your mind, I'd love to try. I too have been burned often by confusing behaviors of tools, and I've done my best to make cargo-semver-checks
not be like that.
I'm also willing to put my money time where my mouth is — I'd be open to maintaining the cargo-semver-checks
CI integration here, including initially adding it to CI. cargo-semver-checks
is already used by tokio
, cargo
, cargo_metadata
and even the tooling that powers the AWS Rust crates, and it's been an extremely low-maintenance addition.
The team behind cargo-semver-checks
recently completed a crater-like run of cargo-semver-checks
against all releases since 2017 of the top 1000 crates on crates.io — 14000+ releases total. This let us catch many bugs before the community runs into them, and also gave us a good sense of semver violations across a large portion of the ecosystem. TL;DR, they happen a lot more than we thought, and they happen to everyone — and we firmly believe "human error" is not the issue.
We're planning to publish a blog post on this ~next week. I'd be happy to share our draft with you today, if you'd like to take a look (no pressure!). We'd obviously welcome any feedback you might have!
(If you aren't interested, I won't be offended if you just close the issue without replying. While I'd love to hear your thoughts, I recognize that's work and not something I'm entitled to — and I already benefit plenty from your work on this crate and many others 🙏)
All righty, I'm willing to give it a go. Thank you for the lovely reply. :)
Awesome!
cargo-semver-checks
is best used like cargo semver-checks && cargo publish
. If you're open to using a manually-triggered GitHub Actions workflow for publishing, copying cargo_metadata
's approach might be easiest. In addition to that release.yml
file, it also needs a crates.io key to be added as a GitHub secret, which is something I can help you through if you'd like.
If you'd prefer to use cargo-semver-checks
on every PR, that's possible but I'd make it a not-required step: by default it will semver-check against the most recent published version, so if you're planning to make a major release and you've already merged a breaking change without updating the version number in Cargo.toml
, that step will stay red until the change is released.
Two other things that might be worth considering:
cargo-semver-checks
heuristically enables all features that don't look like they are unstable / nightly-only / internal-only. This can be overridden with flags if it's not appropriate for this crate — I noticed --default-features
in the terminal session above, presumably because of some inter-feature conflict causing a compilation failure. If so, it may be good to explicitly select which combination of features (possibly, more than one!) should be used for semver-checking.cargo-semver-checks
has a couple of semver-minor lints, which can be disabled if you don't like them. They are things like "#[must_use]
was added in a patch version" which some projects like and others don't (no judgment!). Importantly, none of them are "something new was added to the public API" lints. You can pass --release-type=minor
to make cargo-semver-checks
always think it's checking a minor version bump and therefore skip the semver-minor checks, which is what tokio
does for example.Ping me anytime if anything seems broken or confusing! If you're open to using GitHub Actions for publishing, I'd also be happy to open a PR to add a release.yml
workflow like in cargo_metadata
: https://github.com/oli-obk/cargo_metadata/blob/main/.github/workflows/release.yml
I think starting with a CI step that is allowed to fail is the way to go for now. I don't publish crates from CI. I'm not in theory opposed to it, but let's do one thing at a time. The last thing I want to do is put CI into the critical path for crate releases.
(This is partially why I do so few ripgrep releases. Each release is nearly guaranteed to involve wrangling with CI failures for hours. It's brutal. I do not want to start that across all of my crates unless I get huge wins.)
Sounds good. There's a pre-made GitHub Action that should be easy to throw into a CI job.
I'm also hoping to at some point add a mode to the action for running on a PR and not "pre-publish," and then adjusts the baseline it's comparing against accordingly to be the merge-base commit and not the crates.io published version. That would resolve the "merging breaking changes makes the job stay red until the next release" problem I mentioned.
If that problem 👆 proves to be annoying in your crates' workloads, let me know and I'll prioritize building that functionality. You maintain so many crates for the benefit of the Rust community that empowering you to spend less time wrangling CI would be massively beneficial to the community, and I'd be happy to do what I can toward that 😁
Thank you! :-)
Given #133, and how widespread this crate is to the ecosystem, semver related bugs should not be taken lightly.
Perhaps CI could check for the common offenders using
cargo-semver-check
.Running a test locally,
cargo semver-checks
did catch the bug in 133cc @obi1kenobi