Check for changes to the public API#34
Conversation
|
This includes one for "core2" and one for "alloc", separately because its easy to mess up feature gates with these two. |
|
@Kixunil are you good with this? |
contrib/check-for-api-changes.sh
Outdated
| $CMD --no-default-features | sort --unique > "$API_DIR/no-default-features.txt" | ||
| $CMD | sort --unique > "$API_DIR/default-features.txt" | ||
| $CMD --no-default-features --features=alloc | sort --unique > "$API_DIR/alloc.txt" | ||
| $CMD --no-default-features --features=core2 | sort --unique > "$API_DIR/core2.txt" |
There was a problem hiding this comment.
Could you try sorting with -n? It would read better if the array types were sorted naturally.
There was a problem hiding this comment.
Using -n (or -h) causes the script to produce no output.
There was a problem hiding this comment.
WTF, that's some weird interaction between -n and --unique. You can fix it by writing sort -n | uniq
There was a problem hiding this comment.
So I think we have pushed bash as far as it can reasonably go, bash can't easily sort the following 4 lines as we want AFAIK
impl<'a> hex_conservative::display::DisplayHex for &'a [u8; 9]
impl<'a> hex_conservative::display::DisplayHex for &'a [u8; 8]
pub type [u8; 128]::Err = hex_conservative::HexToArrayError
pub type [u8; 10]::Err = hex_conservative::HexToArrayError
Are you guys ok with a perl script to get the order to be:
impl<'a> hex_conservative::display::DisplayHex for &'a [u8; 8]
impl<'a> hex_conservative::display::DisplayHex for &'a [u8; 9]
pub type [u8; 10]::Err = hex_conservative::HexToArrayError
pub type [u8; 128]::Err = hex_conservative::HexToArrayError
Is there a better tool than perl for this? (FTR neither my perl nor awk are good enough to quickly write this but since we are going to use this script everywhere I can allocate some time to it.)
There was a problem hiding this comment.
Either awk or perl is fine by me. I can read, but not really write, those languages.
There was a problem hiding this comment.
Not a fan of perl but if it's the simplest choice for you I could tolerate it.
cbfecd0 to
5d7eaf5
Compare
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.
5d7eaf5 to
b45bd6e
Compare
|
Added piping through |
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.