Check for changes to the public API #141
Conversation
|
This is the same idea as rust-bitcoin/rust-bitcoin#1880 |
|
Definitely need @clarkmoody to weigh in before we can move forward on this :) |
|
There is a fair bit of context in the PR I link to above @clarkmoody, no rush on this one. Thanks |
tcharding
force-pushed
the
10-17-check-public-api
branch
from
62a188d
to
9788838
Compare
|
cc @clarkmoody can you take a look at this? |
tcharding
force-pushed
the
10-17-check-public-api
branch
from
9788838
to
1a2553b
Compare
|
Rebased and improved to use functions. Also removed the |
|
I can split this up like rust-bitcoin/rust-bitcoin#1880 if its easy for you guys to review. |
tcharding
force-pushed
the
10-17-check-public-api
branch
3 times, most recently
from
119b9f2
to
c82b303
Compare
|
I can't work out why this is failing. Its giving a different sort order than what is in the PR (ie., I'm getting a different order locally to what the VM in CI is getting)? |
|
See discussion on rust-bitcoin/hex-conservative#34 (looks like we didn't do this for rust-secp; surprised we haven't had problems there). I think we want Regardless of reasoning, we should cargo-cult |
tcharding
force-pushed
the
10-17-check-public-api
branch
7 times, most recently
from
8fccd49
to
099b1f8
Compare
|
OMG finally, this sort command got us there |
|
Ugh, bollocks. |
tcharding
force-pushed
the
10-17-check-public-api
branch
from
099b1f8
to
5e31e39
Compare
|
I'm going to debug this in |
We would like to get to a stage where we can commit to the public API. To help us achieve this add a script that generates the public API and checks it against three committed files, one for each feature set: no features, alloc, std. The idea is that with this applied any PR that changes the public API should include a final patch that is just the changes to the api/*.txt files, that way reviewers can discuss the changes without even needing to look at the code, quickly giving concept ACK/NACKs. We also run the script in CI to make sure we have not accidentally changed the public API so that we can be confident that don't break semver during releases. The script can also be used to diff between two release versions to get a complete list of API changes, useful for writing release notes and for users upgrading.
tcharding
force-pushed
the
10-17-check-public-api
branch
from
5e31e39
to
8241316
Compare
|
@clarkmoody are you ok if we merge this? It adds additional burden to contributors with the stated aim of improving our ability to commit to a stable API. |
|
Gentle bump please @clarkmoody, really don't want to merge this without your consent and I'd love to release a non-beta version of this crate |
We would like to get to a stage where we can commit to the public API. To help us achieve this add a script that generates the public API and checks it against three committed files, one for each feature set: no features, alloc, std.
The idea is that with this applied any PR that changes the public API should include a final patch that is just the changes to the api/*.txt files, that way reviewers can discuss the changes without even needing to look at the code, quickly giving concept ACK/NACKs. We also run the script in CI to make sure we have not accidentally changed the public API so that we can be confident that don't break semver during releases. The script can also be used to diff between two release versions to get a complete list of API changes, useful for writing release notes and for users upgrading.
There is a development burden involved if we apply this patch.