ci: add SemVer check workflow to detect API-breaking changes#4476
ci: add SemVer check workflow to detect API-breaking changes#4476jedel1043 merged 4 commits intoboa-dev:mainfrom
Conversation
Test262 conformance changes
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4476 +/- ##
==========================================
+ Coverage 47.24% 57.07% +9.82%
==========================================
Files 476 504 +28
Lines 46892 57378 +10486
==========================================
+ Hits 22154 32747 +10593
+ Misses 24738 24631 -107 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
jedel1043
left a comment
There was a problem hiding this comment.
Some smaller comments, but it's looking almost ready
|
@jedel1043 im imagining there will be times where we will want to make breaking changes, and we don’t want the CI to keep failing and blocking merges. so I guess unlike other CI checks, we may want a way to “skip” this or mark it as optional. I don’t have a solution in mind but it’s worth taking this into account |
|
@jasonwilliams Oh no worries about that. We need to explicitly add the action to the branch rules if we want to require it for PRs, so if we don't add the action, it shouldn't block merges. |
1ae577b to
6fa5b6f
Compare
|
Now that we published 0.21, we can try to merge this. Gonna update the branch to see that the action passes, then merge after that. |
Already addressed all comments.
| - name: Set environment | ||
| run: | | ||
| target=$(rustc -vV | awk '/^host/ { print $2 }' | tr [:lower:] [:upper:] | tr '-' '_') | ||
| echo "CARGO_TARGET_${target}_RUSTFLAGS=$W_FLAGS" >> $GITHUB_ENV |
There was a problem hiding this comment.
Hey, I'm the cargo-semver-checks maintainer 👋
Hope you don't mind me giving you a heads-up here: the -D warnings set here is ineffective and explicitly overridden by cargo-semver-checks using --cap-lints=allow.
This is because -D warnings is indiscriminate and can cause cargo-semver-checks to fail to build the baseline version, not just the current version being checked. For example, we've had situations where:
- a dependency X published a new non-major version
- the new version caused a new warning to appear in all versions of crate Y that depends on X
- downstream crate Z, which depends on Y but doesn't know about X, cannot run
cargo-semver-checksbecause of the new warnings upstream
Wanting to deny warnings is understandable, but it's best to do that in jobs other than cargo-semver-checks because this job is the only one that'll need to build older versions of your crates, not just main.
This Pull Request fixes/closes #4471.
It changes the following: