-
-
Notifications
You must be signed in to change notification settings - Fork 371
ref: Wrap SentryDispatchQueueWrapper in Swift #5472
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
a19cd41 to
a12c848
Compare
philipphofmann
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this looks way better than #5459 cause it's less risky. Thanks.
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 5cfc768 | 1220.74 ms | 1245.06 ms | 24.32 ms |
| e64d3d4 | 1241.90 ms | 1260.10 ms | 18.20 ms |
| f92cfa9 | 1217.94 ms | 1240.06 ms | 22.12 ms |
| 63ac649 | 1192.10 ms | 1216.78 ms | 24.68 ms |
| d637379 | 1226.43 ms | 1250.77 ms | 24.34 ms |
| f92cfa9 | 1228.45 ms | 1251.33 ms | 22.88 ms |
| 8ea5293 | 1242.70 ms | 1262.25 ms | 19.55 ms |
| 35c962f | 1207.61 ms | 1235.90 ms | 28.29 ms |
| 2691350 | 1224.92 ms | 1255.82 ms | 30.90 ms |
| 2b02431 | 1229.63 ms | 1248.98 ms | 19.35 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 5cfc768 | 23.75 KiB | 850.73 KiB | 826.98 KiB |
| e64d3d4 | 23.75 KiB | 855.37 KiB | 831.62 KiB |
| f92cfa9 | 23.75 KiB | 855.38 KiB | 831.63 KiB |
| 63ac649 | 23.75 KiB | 855.38 KiB | 831.63 KiB |
| d637379 | 23.75 KiB | 855.38 KiB | 831.63 KiB |
| f92cfa9 | 23.75 KiB | 855.38 KiB | 831.62 KiB |
| 8ea5293 | 23.75 KiB | 852.24 KiB | 828.49 KiB |
| 35c962f | 23.75 KiB | 854.77 KiB | 831.02 KiB |
| 2691350 | 23.75 KiB | 850.73 KiB | 826.98 KiB |
| 2b02431 | 23.75 KiB | 850.73 KiB | 826.98 KiB |
Previous results on branch: wrapDispatchQueueWrapperSwift
Startup times
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| b1424c9 | 1223.12 ms | 1245.04 ms | 21.92 ms |
| 4cb5674 | 1224.50 ms | 1248.70 ms | 24.20 ms |
| a657085 | 1226.92 ms | 1248.04 ms | 21.12 ms |
| caa31a0 | 1227.37 ms | 1248.69 ms | 21.33 ms |
| ea47880 | 1226.88 ms | 1251.48 ms | 24.60 ms |
| 02d61c0 | 1235.37 ms | 1254.91 ms | 19.55 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| b1424c9 | 23.75 KiB | 853.28 KiB | 829.53 KiB |
| 4cb5674 | 23.75 KiB | 860.80 KiB | 837.05 KiB |
| a657085 | 23.75 KiB | 857.97 KiB | 834.22 KiB |
| caa31a0 | 23.75 KiB | 857.51 KiB | 833.76 KiB |
| ea47880 | 23.75 KiB | 857.96 KiB | 834.21 KiB |
| 02d61c0 | 23.75 KiB | 853.27 KiB | 829.53 KiB |
55c28be to
b636afc
Compare
philipphofmann
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, when CI is green.
6cbdd95 to
34a8573
Compare
🚨 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:
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5472 +/- ##
=============================================
+ Coverage 85.915% 86.289% +0.373%
=============================================
Files 400 402 +2
Lines 34720 34908 +188
Branches 14959 15144 +185
=============================================
+ Hits 29830 30122 +292
+ Misses 4847 4743 -104
Partials 43 43
... and 30 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
This is a redo of #5459 in a way that makes no changes to the implementation, just creates the Swift wrapper that replaces the ObjC version to allow further migration to Swift
#skip-changelog