ci: add SentryCrash import ratchet to PR checks#7723
Conversation
- Count direct SentryCrash imports from SDK sources - Fail if count exceeds baseline of 59 - Add check-sentrycrash-imports Makefile target
- Add sentrycrash-import-ratchet job to fast-pr-checks workflow - Run on ubuntu-latest (only needs grep, no Xcode) - Gate fast-checks-required on ratchet passing
Semver Impact of This PR🟢 Patch (bug fixes) 📋 Changelog PreviewThis is how your changes will appear in the changelog. This PR will not appear in the changelog. 🤖 This preview updates automatically when you update the PR. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Grep pattern misses indented imports and includes
- Updated grep to use extended regex matching both indented preprocessor directives and #include statements, increasing coverage from 59 to 84 imports.
- ✅ Fixed: Dead grep filter never matches any results
- Removed the ineffective grep filter and replaced with direct wc -l count.
Or push these changes by commenting:
@cursor push f223d5e34f
Preview (f223d5e34f)
diff --git a/scripts/check-sentrycrash-imports.sh b/scripts/check-sentrycrash-imports.sh
--- a/scripts/check-sentrycrash-imports.sh
+++ b/scripts/check-sentrycrash-imports.sh
@@ -10,11 +10,11 @@
# shellcheck source=./ci-utils.sh disable=SC1091
source "$SCRIPT_DIR/ci-utils.sh"
-MAX_IMPORTS=59
+MAX_IMPORTS=84
-count=$(grep -rn '#import.*SentryCrash' Sources/Sentry Sources/Swift \
+count=$(grep -rEn '#[[:space:]]*(import|include).*SentryCrash' Sources/Sentry Sources/Swift \
--include='*.m' --include='*.h' --include='*.c' --include='*.mm' \
- | grep -vc 'Sources/SentryCrash/' \
+ | wc -l \
| tr -d ' ')
if [ "$count" -gt "$MAX_IMPORTS" ]; then
@@ -23,9 +23,8 @@
log_error "Use the SentryCrashReporter protocol instead."
echo ""
log_notice "Offending imports:"
- grep -rn '#import.*SentryCrash' Sources/Sentry Sources/Swift \
- --include='*.m' --include='*.h' --include='*.c' --include='*.mm' \
- | grep -v 'Sources/SentryCrash/'
+ grep -rEn '#[[:space:]]*(import|include).*SentryCrash' Sources/Sentry Sources/Swift \
+ --include='*.m' --include='*.h' --include='*.c' --include='*.mm'
exit 1
fiThis Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7723 +/- ##
=============================================
+ Coverage 85.371% 85.388% +0.017%
=============================================
Files 487 487
Lines 28985 28985
Branches 12552 12551 -1
=============================================
+ Hits 24745 24750 +5
+ Misses 4191 4187 -4
+ Partials 49 48 -1 see 7 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
🚨 Detected changes in high risk code 🚨High-risk code can easily blow up and is hard to test. We had severe bugs in the past. Be extra careful when changing these files, and have an extra careful look at these:
|
Sentry Build Distribution
|


📜 Description
Add a CI ratchet that prevents the number of direct SentryCrash header imports from SDK source files from increasing beyond the current baseline of 59.
scripts/check-sentrycrash-imports.sh— counts#import.*SentryCrashinSources/SentryandSources/Swift, fails if above baselineMakefile— addscheck-sentrycrash-importstarget.github/workflows/fast-pr-checks.yml— addssentrycrash-import-ratchetjob gated onrun_unit_tests_for_prs, runs onubuntu-latest💡 Motivation and Context
Part of the SentryCrash decoupling project. As we isolate SentryCrash behind protocols, this ratchet ensures coupling doesn't regress. The baseline should be decreased as isolation work progresses.
#skip-changelog
💚 How did you test it?
make check-sentrycrash-importspasses locally (59 / 59)📝 Checklist
sendDefaultPIIis enabled.Fixes: #7713