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

Added experimental semver-check pipeline #708

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Rob2309
Copy link
Contributor

@Rob2309 Rob2309 commented Jan 21, 2023

This PR is intended to resolve #694

@Rob2309 Rob2309 marked this pull request as draft January 24, 2023 09:51
@dbr
Copy link
Contributor

dbr commented Jan 30, 2023

Excellent, thanks!

I think we should probably check ~all the crates (the renderers etc)

Only other thing is we should cache (or at least reuse) the installation of cargo-semver-checks as the build takes a while and repeats for each of the matrix.crate - looks like this is in-progress with upstream obi1kenobi/cargo-semver-checks-action#21 - so probably just ignore the rebuilding for now, and just update when that PR is released

@Rob2309
Copy link
Contributor Author

Rob2309 commented Jan 30, 2023

I noticed that the semver check currently checks against v0.9.0, which probably should not be the case right? That means we would need to bump the Cargo.toml version to the next version right after releasing one. Maybe this can be changed though

@obi1kenobi
Copy link

obi1kenobi commented Feb 1, 2023

👋 Hi! cargo-semver-checks author here. Thanks for trying cargo-semver-checks!

I think this might be a bug in the v1 GitHub Action, which was written before cargo-semver-checks could by itself look up the correct baseline version to use based on what's on crates.io. The v1 GitHub Action's logic has some known issues, and we're excited to be removing it completely in the v2 of the action in the coming couple of weeks.

You shouldn't have to bump versions right after publishing. The intended behavior (tested in cargo-semver-checks, not the v1 Action) is:

  • If the Cargo.toml's version is not yet on crates.io, then that's assumed to be the about-to-be-published version. It is compared against the highest-numbered non-yanked non-prerelease version on crates.io that is less than the Cargo.toml version.
  • If the Cargo.toml's version is already on crates.io, then the local code is assumed to be a patch version on top of what Cargo.toml has. It is then compared against the Cargo.toml version as published on crates.io.

To get this behavior today, you could replace the GitHub Action with just cargo install --locked cargo-semver-checks && cargo semver-checks check-release <any-flags-you-need>. Then once the v2 Action is ready for prime-time, I'd recommend switching back to the action to get a speed boost thanks to things like prebuilt binaries and (future feature) baseline rustdoc caching.

Sorry for the trouble with the old GitHub Action, we're on that! In general, please don't be shy about opening issues about points of friction you run into, even if minor!

@Rob2309
Copy link
Contributor Author

Rob2309 commented Feb 1, 2023

That sounds very promising :) Since there is no big pressure to introduce this, we might just want to wait for the action to be released.
Since you mentioned opening issues, should we still open an issue for the v1 action?

@obi1kenobi
Copy link

Since you mentioned opening issues, should we still open an issue for the v1 action?

No need — I just checked the latest cargo-semver-checks version and it correctly picks the published 0.10.0 version as a baseline, so the problem is guaranteed to go away in the v2 action. I think the v1 action probably just has a < instead of a <= somewhere :)

@dbr
Copy link
Contributor

dbr commented Feb 3, 2023

Excellent 🥳

Since there is no big pressure to introduce this, we might just want to wait for the action to be released

Yep agreed, lets await the V2 action

@dbr
Copy link
Contributor

dbr commented Mar 22, 2023

Oh, I didn't realize teh v2 action hadn't been released yet (discussed obi1kenobi/cargo-semver-checks-action#26 )

@obi1kenobi
Copy link

Oof, sorry about that! I'll check in with the person working on the action and see if we can get it to a release-able state soon. I definitely didn't intend to leave it in its current non-working state for this long... 😅

@obi1kenobi
Copy link

We just released v2 of the action as well as a new massively-faster cargo-semver-checks version — up to 2354x faster measured on a large crate.
https://github.com/obi1kenobi/cargo-semver-checks-action/releases/tag/v2.0

Full docs here:
https://github.com/obi1kenobi/cargo-semver-checks-action#cargo-semver-checks-action

The crate-name argument is now called package (matching what cargo calls it) and accepts a comma-separated list as well, if you'd like to check both crates in one job. With the new optimizations, the "build rustdoc" portion of the runtime dominates the actual checking, so running both crates in one job might be more efficient if they depend on each other or have significant dependencies in common.

If you run into any issues migrating onto the new action, please give me a ping and I'd be happy to help!

@sanbox-irl
Copy link
Member

@Rob2309 do you have the bandwidth to update this fella with the new changes?

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.

Setup cargo-semver-checks or similar in CI
4 participants