fix(ci): always run cargo-public-api in semver-level.sh - #2072
Conversation
cargo-semver-checks misses parameter-type changes on non-generic functions (no function_parameter_type_changed lint), returning minor when the change is actually breaking. Run cargo-public-api unconditionally and take the higher of the two signals via max_level so removed/changed signatures are caught even when cargo-semver-checks already flagged the crate at minor. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2072 +/- ##
==========================================
- Coverage 73.44% 73.42% -0.02%
==========================================
Files 465 465
Lines 77949 77949
==========================================
- Hits 57248 57233 -15
- Misses 20701 20716 +15
🚀 New features to boost your workflow:
|
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: 0ed1b5e | Docs | Datadog PR Page | Give us feedback! |
Artifact Size Benchmark Reportaarch64-alpine-linux-musl
aarch64-unknown-linux-gnu
libdatadog-x64-windows
libdatadog-x86-windows
x86_64-alpine-linux-musl
x86_64-unknown-linux-gnu
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5e27ecfbb0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if grep -q "Removed items from the public API$" <<< "$PUBLIC_API_OUTPUT" \ | ||
| && ! grep -A 2 "^Removed items from the public API$" <<< "$PUBLIC_API_OUTPUT" | grep -q "^(none)$"; then | ||
| public_api_level="major" |
There was a problem hiding this comment.
When cargo-semver-checks already returns minor (for example, an added public item), this new path now runs cargo-public-api and promotes any removed textual item to major. I checked .github/workflows/pr-title-semver-check.yml and .github/workflows/release-proposal-dispatch.yml; both install cargo-public-api@0.50.2, and the cargo-public-api v0.52.0 changelog says it stopped including function parameter names by default to avoid rename diffs, which means the pinned version can report a public function parameter rename as Removed+Added even though Rust callers are not broken. A PR that adds an API and renames a parameter will now be forced to major instead of the semver-checks minor result.
Useful? React with 👍 / 👎.
| elif grep -q "Added items to the public API$" <<< "$PUBLIC_API_OUTPUT" \ | ||
| && ! grep -A 2 "^Added items to the public API$" <<< "$PUBLIC_API_OUTPUT" | grep -q "^(none)"; then |
There was a problem hiding this comment.
When cargo-semver-checks returns minor and the extra cargo-public-api run reports a signature diff under Changed items in the public API, this parser leaves public_api_level as none because it only inspects Removed and Added sections. I checked cargo-public-api's CLI/source and it treats changed as its own diff category (--deny=changed), so the non-generic parameter-type changes this patch is meant to catch can still be released as minor instead of major unless they happen to render as removals.
Useful? React with 👍 / 👎.
…ublic-api's Changed section
# What does this PR do? Modify `semver-level.sh` to check the output of `cargo-public-api` Upgrade `cargo-public-api` to `0.52.0` Upgrade `cargo-semver-checks` to `0.48.0` # Motivation `cargo-semver-checks` misses parameter-type changes on non-generic functions (no function_parameter_type_changed lint), returning minor when the change is actually breaking. Run `cargo-public-api` unconditionally and take the higher of the two signals via max_level so removed/changed signatures are caught even when `cargo-semver-checks` already flagged the crate at minor. --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> Signed-off-by: Taegyun Kim <taegyun.kim@datadoghq.com>
3 participants
What does this PR do?
Modify
semver-level.shto check the output ofcargo-public-apiUpgrade
cargo-public-apito0.52.0Upgrade
cargo-semver-checksto0.48.0Motivation
cargo-semver-checksmisses parameter-type changes on non-generic functions (no function_parameter_type_changed lint), returning minor when the change is actually breaking. Runcargo-public-apiunconditionally and take the higher of the two signals via max_level so removed/changed signatures are caught even whencargo-semver-checksalready flagged the crate at minor.