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
itaybre
requested review from
noahsmartin,
philipphofmann and
philprime
as code owners
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. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Autofix Details
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.186% 85.467% +0.281%
=============================================
Files 490 487 -3
Lines 29520 29651 +131
Branches 12761 12850 +89
=============================================
+ Hits 25147 25342 +195
+ Misses 4322 4258 -64
Partials 51 51 see 37 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
📲 Install BuildsiOS
|
…rycrash-import-ratchet
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| ae8cece | 1216.83 ms | 1251.37 ms | 34.55 ms |
| adef457 | 1229.45 ms | 1262.67 ms | 33.22 ms |
| 1887b2e | 1218.51 ms | 1243.06 ms | 24.55 ms |
| 6c524d8 | 1217.83 ms | 1254.02 ms | 36.20 ms |
| 65c13c5 | 1234.47 ms | 1263.10 ms | 28.63 ms |
| 1c5ecda | 1219.35 ms | 1253.76 ms | 34.41 ms |
| c958e9d | 1219.05 ms | 1256.44 ms | 37.40 ms |
| c5c7a29 | 1223.14 ms | 1258.55 ms | 35.41 ms |
| a7c42d9 | 1217.25 ms | 1253.98 ms | 36.73 ms |
| 29d546e | 1224.06 ms | 1257.05 ms | 32.98 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| ae8cece | 24.14 KiB | 1.15 MiB | 1.13 MiB |
| adef457 | 24.14 KiB | 1.15 MiB | 1.13 MiB |
| 1887b2e | 24.14 KiB | 1.15 MiB | 1.13 MiB |
| 6c524d8 | 24.14 KiB | 1.15 MiB | 1.12 MiB |
| 65c13c5 | 24.14 KiB | 1.15 MiB | 1.12 MiB |
| 1c5ecda | 24.14 KiB | 1.15 MiB | 1.12 MiB |
| c958e9d | 24.14 KiB | 1.15 MiB | 1.13 MiB |
| c5c7a29 | 24.14 KiB | 1.15 MiB | 1.13 MiB |
| a7c42d9 | 24.14 KiB | 1.15 MiB | 1.13 MiB |
| 29d546e | 24.14 KiB | 1.15 MiB | 1.13 MiB |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 8241081. Configure here.
🚨 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:
|
2 participants
📜 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 86.
💡 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 (86 / 86)📝 Checklist
sendDefaultPIIis enabled.Fixes: #7713