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

Add cargo-semver-checks to automate checking for breaking changes. #2145

Open
JonathanWoollett-Light opened this issue Oct 2, 2023 · 4 comments

Comments

@JonathanWoollett-Light
Copy link
Contributor

JonathanWoollett-Light commented Oct 2, 2023

Add cargo-semver-checks to automate checking for breaking changes.

This may allow removing:

  • A change log has been added if this PR modifies nix's API

from the PR template, here: https://github.com/nix-rust/nix/blob/master/.github/PULL_REQUEST_TEMPLATE.md?plain=1#L7

@SteveLauC
Copy link
Member

This is a nice tool!

Just gave a brief look at its doc, if I understand correctly, after integrating it to our CI, a PR would fail the test if the changes made in this PR have semver violations.

What if semver violation is allowed, say we are going to bump our major version in the next release

@JonathanWoollett-Light
Copy link
Contributor Author

JonathanWoollett-Light commented Oct 4, 2023

a PR would fail the test if the changes made in this PR have semver violations.

It would be if the PR has semver violations relative to the last release on crates.io.

What if semver violation is allowed, say we are going to bump our major version in the next release

You would bump the version in the 1st commit since the last release that introduces a breaking change.

@asomers
Copy link
Member

asomers commented Oct 4, 2023

You would bump the version in the 1st commit since the last release that introduces a breaking change.

This would have an unintended consequence of making it more difficult for downstream crates to test new Nix changes. Right now, Nix never bumps its version number until a release. That means that a downstream crate can use a [patch.crates-io] section to override the Nix dependency temporarily, to test some change or another. I find this mechanism very useful. However, Cargo will ignore the patch section if the patched crate's version is different than the one in Cargo.lock. This means that if we adopt cargo-semver-checks, then downstream crates would have to both add a patch section and change the version in their [dependencies] section. Worse, Cargo won't use the patched Nix transitively in any of that crate's other dependencies.

IMHO the benefit of cargo-semver-checks is not worth the cost.

@asomers
Copy link
Member

asomers commented Oct 4, 2023

That said, this tool looks like it would be worthwhile to use in the release procedure, just not in CI.

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

No branches or pull requests

3 participants