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

Check for API breaking changes in CI #1875

Open
tcharding opened this issue May 25, 2023 · 5 comments · May be fixed by #2713
Open

Check for API breaking changes in CI #1875

tcharding opened this issue May 25, 2023 · 5 comments · May be fixed by #2713
Labels
1.0 Issues and PRs required or helping to stabilize the API

Comments

@tcharding
Copy link
Member

tcharding commented May 25, 2023

As we move towards a 1.0 release it would be nice to check CI that we don't make breaking changes. Consider using

@tcharding tcharding added the 1.0 Issues and PRs required or helping to stabilize the API label May 25, 2023
@apoelstra
Copy link
Member

We should definitely be using cargo-semver-checks -- though bearing in mind that it's still very incomplete and we can't blindly trust an "ok" from it.

cargo-public-api looks pretty interesting. I like the idea of committing the public API in contrib/ or somethig and then diffing against that in CI, so that any API changes become visible (and easy to check for after-the-fact). And maybe there are lints we could run against the text file to check if e.g. all errors impl Debug and std::Error and so on. We'd have to experiment a bit to see how well it handles our feature-gating etc.

@tcharding
Copy link
Member Author

Whats the status of this one crew, do we want it in before next release? The one on bech32 has stalled.

ref: rust-bitcoin/rust-bech32#141

@Kixunil
Copy link
Collaborator

Kixunil commented Jan 24, 2024

I'm not sure there should be rush but I want it eventually, definitely before 1.0 of any crate. Doing it sooner has the advantage of being able to auto-label PRs.

@apoelstra
Copy link
Member

Let's just rebase #1880 and do it here, since Kix is onboard.

The history here is that we tried opening #1880 and got no reply from Kix after several weeks and were unable to move forward. So we switched to doing it in leaf crates where we didn't need Kix's approval (e.g. in rust-secp rust-bitcoin/rust-secp256k1#656 and in rust-bech32). Then in rust-bech32 we have gotten no reply from Clark for several weeks :).

For what it's worth, it's worked great in rust-secp ... but that's not worth much since the API for rust-secp never changes :).

But since Kix is online and in favor of this, I think we should just pull the trigger instead of waiting on rust-bech32.

@tcharding
Copy link
Member Author

Then in rust-bech32 we have gotten no reply from Clark for several weeks

Just to clear the respond Clark did respond, it was back waiting on @Kixunil (because of merge strategy proposed by me). No one is really at fault here.

Anyways, I'll rebase and YOLO - if there are too many teething problems we can always revert.

@tcharding tcharding added this to the 0.32.0 milestone Feb 5, 2024
@tcharding tcharding removed this from the 0.32.0 milestone Feb 29, 2024
@tcharding tcharding linked a pull request Apr 24, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.0 Issues and PRs required or helping to stabilize the API
Projects
None yet
3 participants