Add script to create API files#2986
Conversation
Add a script that can be used locally to check the API changes of a PR. Assumes you are on the PR branch and that the branch is rebase cleanly on top of tip of `master`. Does the following: - Grabs a list of the commits back to `master` - Creates a new tmp branch on the tip of master - For each commit do: - create the API files - cherry pick the commit - Add a final patch with API file changes
|
Waist of infrastructure resources running CI on this PR. |
|
I don't get it. Are we getting the |
|
No this is just so folk that like it can run it locally and use the text files to review and/or check their own PRs. |
|
This may be better as an external project since you probably don't need the review rigor of this crate and I don't really want to block you by not reviewing it. Or just post it into discussion as I did with the bisect script. |
| done | ||
|
|
||
| # Reverse the commit list to process from oldest to newest | ||
| commits=$(echo "$commits" | tac) |
There was a problem hiding this comment.
In f84fb80:
You can just use git rev-list --reverse.
| # Find commits on current branch that are not in master | ||
| commits=$(git rev-list master.."$current_branch") | ||
|
|
||
| git checkout -b "$TEMP_BRANCH" master || exit 1 |
There was a problem hiding this comment.
In f84fb80:
This exit 1 doesn't do anything except throwing away the return value from git, since you have set -e on the script.
|
|
||
| if [[ $(git status --porcelain api) ]]; then | ||
| git add -A | ||
| git commit -m "api: Run check-for-api-changes.sh" -n |
There was a problem hiding this comment.
In f84fb80:
I'd also change the author field and add --no-gpg-sign.
|
In f84fb80: Should check that the repo isn't dirty and that the list of commits isn't empty. Alternately, you could just take a PR branch on the command-line rather than doing |
|
Thanks for the review @apoelstra I'll work in your suggestions.
Ah good point, I rekon I'll move this to |
|
FTR this is low priority now, I wanted to have something available that @apoelstra could use since I said I would. This is usable now even if not mergable. |
In an effort to check off items in the Rust API guidelines checklist (rust-bitcoin#3632) add an integration test file that tests: - The location of re-exports for various typical usage styles. - Regressions in the API surface (things being accidentally moved). - All public types implement Debug (C-DEBUG). - For all non-error types: - `Debug` representation is never empty (C-DEBUG-NONEMPTY) - For all error types: - Derive standard traits as defined by `rust-bitcoin` policy. I used the `cargo check-api` script we have laying around from ages ago (rust-bitcoin#2986) to parse `units` and get a list of the public types.
7b8369f units: Add integration test of API surface (Tobin C. Harding) Pull request description: In an effort to check off items in the Rust API guidelines checklist (#3632) add an integration test file that tests: - The location of re-exports for various typical usage styles. - Regressions in the API surface (things being accidentally moved). - All public types implement Debug (C-DEBUG). - For all non-error types: - `Debug` representation is never empty (C-DEBUG-NONEMPTY) - For all error types: - Derive standard traits as defined by `rust-bitcoin` policy. - All data structures implement `serde` traits (C-SERDE). I used the `cargo check-api` script we have laying around from ages ago (#2986) to parse `units` and get a list of the public types. ACKs for top commit: apoelstra: ACK 7b8369f; successfully ran local tests Tree-SHA512: 6fa2a61f6b67a6b5a56950b87e6df68b761b69bd2a49e7aac48aa238cb84441ce04acd85097d28ae4055058052a7491eccda3da218812149a896e548b4018aaa
|
We can get rid of this since we merged #3682 |
Add a script that can be used locally to check the API changes of a PR.
Assumes you are on the PR branch and that the branch is rebase cleanly on top of tip of
master.Does the following:
master