-
-
Notifications
You must be signed in to change notification settings - Fork 372
ref: SentryClient and SentryHub in Swift #6627
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
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #6627 +/- ##
=============================================
- Coverage 85.507% 85.178% -0.330%
=============================================
Files 451 452 +1
Lines 27566 27669 +103
Branches 12069 12118 +49
=============================================
- Hits 23571 23568 -3
- Misses 3950 4055 +105
- Partials 45 46 +1
... and 2 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
e5f1cb6 to
376f35a
Compare
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| b41e6a7 | 1201.65 ms | 1246.12 ms | 44.47 ms |
| daeb716 | 1215.41 ms | 1246.52 ms | 31.11 ms |
| 535ebd9 | 1194.59 ms | 1219.84 ms | 25.26 ms |
| b709887 | 1193.52 ms | 1216.74 ms | 23.22 ms |
| e98d6f5 | 1228.51 ms | 1258.85 ms | 30.34 ms |
| 76f74df | 1238.29 ms | 1261.22 ms | 22.94 ms |
| 0b6776b | 1230.18 ms | 1262.06 ms | 31.88 ms |
| 9389467 | 1218.62 ms | 1244.86 ms | 26.24 ms |
| 86eb6d5 | 1224.95 ms | 1251.82 ms | 26.87 ms |
| 561321a | 1229.60 ms | 1252.55 ms | 22.95 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| b41e6a7 | 23.75 KiB | 908.03 KiB | 884.28 KiB |
| daeb716 | 23.75 KiB | 928.16 KiB | 904.41 KiB |
| 535ebd9 | 23.75 KiB | 1008.67 KiB | 984.92 KiB |
| b709887 | 23.75 KiB | 1.01 MiB | 1016.14 KiB |
| e98d6f5 | 23.75 KiB | 933.03 KiB | 909.28 KiB |
| 76f74df | 23.75 KiB | 879.61 KiB | 855.86 KiB |
| 0b6776b | 23.75 KiB | 968.23 KiB | 944.49 KiB |
| 9389467 | 23.75 KiB | 866.51 KiB | 842.76 KiB |
| 86eb6d5 | 23.74 KiB | 1.02 MiB | 1016.53 KiB |
| 561321a | 23.75 KiB | 963.18 KiB | 939.43 KiB |
48ac80a to
bed31ff
Compare
bbfbdc6 to
5169bbd
Compare
5169bbd to
1b852ec
Compare
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, thank you for doing this.
This PR looks big in the line count, but only because it renamed some things. The real changes are just SentryId.h/.m, SentryClient.swift, and SentryHub.swift. All of these are pretty small.
Refactors the public interface of SentryClient and SentryHub to be in Swift. The implementation doesn't change at all I keep the old old classes around and just renamed them to SentryClientInternal and SentryHubInternal. This unblocks using SPM without any risk of a big Swift migration, and lets us migrate them when we want without affecting the public API.
Also converts SentryId to ObjC which is required for SPM, and lets us delete SentryIdWrapper. We can convert it to Swift again once all of the classes depending on it in the public API are also in Swift. Basically, this should be the last part of the public API to be in Swift.
#skip-changelog
Closes #6630